Fix count when parsing bottom command

[joelim-work]

Fixes #1239

To reproduce, type :bottom into the command line, or alternatively define a custom mapping like map b bottom in the config file.

Since #1196, the top and bottom commands take a count which specifies which line to jump to. For this to work, the default counts for these two commands have to be changed to 0 (the usual default count for commands is 1), to enable jumping to the first line. This change modifies to default count to be 0 specifically for these two commands. Edit: I changed my mind to use the alternate solution below.

An alternate solution would be to have all the commands use a default count of 1, and just not allow the user to jump to the first line, since top does this already.

[joelim-work]

I decided to just keep things simple and use a default count of 1 to match all the other commands. The only difference is that 1G jumps to the bottom line instead of the top like Vim, but I think it's an impractical use case (normally you would prefer to use gg here).

I think this is a better approach than making changes to the command parsing logic. For reference, here is the diff for the previous implementation:

diff --git a/parse.go b/parse.go
index e6abeb0..72cbdbc 100644
--- a/parse.go
+++ b/parse.go
@@ -221,21 +221,25 @@ func (p *parser) parseExpr() expr {
 		default:
 			name := s.tok
 
 			var args []string
 			for s.scan() && s.typ != tokenSemicolon {
 				args = append(args, s.tok)
 			}
 
 			s.scan()
 
-			result = &callExpr{name, args, 1}
+			count := 1
+			if name == "top" || name == "bottom" {
+				count = 0
+			}
+			result = &callExpr{name, args, count}
 		}
 	case tokenColon:
 		s.scan()
 
 		var exprs []expr
 		if s.typ == tokenLBraces {
 			s.scan()
 			for {
 				e := p.parseExpr()
 				if e == nil {

[ilyagr]

I don't feel very strongly about this, and I'm having a little trouble making up my mind, but here are two somewhat opposing thoughts.

One thought is that vim's behavior is illogical here, and we shouldn't follow it strictly.

Here's the best idea I have at the moment:

• Make the default :bottom/G command always go to the bottom. If its count is not 1, it should print an error message.
• Make the top command go to count-th line. So 5gg would go to the 5th line.
• For people who can't unlearn their Vim muscle memory, create a new bottom-or-goto-line command that behaves like what you implemented here (go to bottom if count is 1).
• (Optionally) make :15<ret> go to 15th line (but this would be a separate feature).

Alternatively, we could map G to vim-bottom-or-goto-line and map ge to bottom like Kakoune.

Do you have a preference?

Note that this doesn't block this PR, especially if we go with the G maps to bottom-or-goto-line option. In that case, the other functionality I discussed could go into a separate PR.

---

Another, diametrically opposed, alternative is to essentially go back to your original approach: have all commands default to count 0 (or make the count signed and use -1?), and have most commands automatically convert that to 1. I'm going back and forth in my mind about whether this means we should forbid the user from entering the count of 0 themselves. We do allow the user to enter a count of 0 now. If we continue allowing this, and use 0 as opposed to -1 as the default, then they should probably observe that it is equivalent to the count of 1 for most but not all commands.

[joelim-work]

Thanks for your comments @ilyagr

First off, I want to say that my preference is to keep things simple. This applies to both the code implementation, and the set of features offered to users. I do acknowledge that that the behavior of 1G will be different to Vim and can be considered a bug, but in my opinion it won't matter much since 1G isn't really a use case when gg is available, and that it is more important to fix #1239 than to worry about what the behaviour of 1G should be (this can be addressed later if necessary).

Regarding your suggestion about bottom-or-goto-line, I don't agree with having G ignore the count and unconditionally move to the bottom, at least not by default. That is to say, 5gg and 5G should have the same behaviour. I would also like to avoid introducing multiple variants of the bottom command to users just to iron out a small wrinkle.

I think your other suggestion about making all the commands default to a count of 0 is more viable though. For many commands, it won't matter since they ignore the count and just execute once. And for the few commands that do care about the count (e.g. up, scroll-up, updir, jump-prev), they can convert the count from a 0 to a 1 first. But the current implementation also makes sense - pressing the key or typing the command directly implies that you want to execute the command exactly once, so therefore the default count is set to 1. I think this can be discussed further, but perhaps separately from this PR, which is intended to be a quick patch.

I should also point out that although lf allows the user to enter a count of 0, it will actually get ignored and the default count from the mapping will be used instead. My understanding is that up until now, a command count was interpreted as a literal count of how many times the command should be logically applied, as opposed to a modifier argument like what is required for the top/bottom commands, and that 0 was never considered to be a valid value for the count.

lf/ui.go

Lines 1175 to 1179 in f04401b

	expr := gOpts.keys[string(ui.keyAcc)]
	if e, ok := expr.(*callExpr); ok && count != 0 {
	expr = &callExpr{e.name, e.args, e.count}
	expr.(*callExpr).count = count

[joelim-work]

I have added a comment in the code to point out the difference in behaviour of 1G when compared to Vim.

[gokcehan]

I agree we should simply merge the current patch a quick fix and discuss other possible changes separately (either here or in another issue/PR). Thanks @joelim-work for the patch and thanks @ilyagr for the review and discussion.