jesseduffield / lazygit

simple terminal UI for git commands
MIT License
52.66k stars 1.84k forks source link

The {{line}} variable has stopped working #2589

Open DusanLesan opened 1 year ago

DusanLesan commented 1 year ago

Describe the bug The {{line}} variable available for custom commands has stopped being replaced into real number when command is executed

To Reproduce Steps to reproduce the behavior:

  1. Add openCommand: 'notify-send "{{filename}}:{{line}}"' into your config
  2. Open changes view of any file
  3. Verify the text sent to the command

Expected behavior The {{line}} used to be a valid way to indicate to text editor a line to focus after opening

Screenshots Notice the text of notification here: HDMI-1-20230503-123724

Version info: commit=v0.38.1, build date=2023-05-02T09:09:52Z, build source=binaryRelease, version=0.38.1, os=linux, arch=amd64, git version=2.40.1

git version 2.40.1

mark2185 commented 1 year ago

I believe that config option was changed from openCommand to just open in the latest major release.

stefanhaller commented 1 year ago

Yes, but openCommand is still supported as a legacy fallback for now.

The more important change is that open (and openCommand) no longer support the {{line}} variable. This is by design; the "o" command is meant to simulate opening a file as if it was double-clicked in the Finder or Explorer, and "double-clicking a file at a certain line" doesn't make sense. Only the "e" command supports the {{line}} variable now.

See https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#configuring-file-editing.

DusanLesan commented 1 year ago

@mark2185 I did try open command instead of openCommand but posted like this since I did not see the difference. @stefanhaller I am sad to see open command losing this functionality as I liked having e for vim and o for vs code since I use both depending on language. Also I would like to have it available from changes view context for custom commands

If it is as designed, feel free to close this ticket.

stefanhaller commented 1 year ago

I'll leave this open, this is valuable input. @jesseduffield, there was no real reason to remove {{line}} support for the open command, we only did it in an attempt to make things clearer. I suppose it wouldn't hurt to put it back in (leaving everything else the way it is). What do you think?

Also I would like to have it available from changes view context for custom commands

Yes, this sounds useful, but seems unrelated to me. This didn't work in 0.37 either, right?

DusanLesan commented 1 year ago

Yes, this sounds useful, but seems unrelated to me. This didn't work in 0.37 either, right?

Right. I am not aware of it ever working from changed files context.

I look forward to the resolution of this issue. I hope {{line}} is returned for users that want that and that you can hardcode line 1 to achieve designed behavior in default commands.

stefanhaller commented 1 year ago

and that you can hardcode line 1 to achieve designed behavior in default commands.

Right, now that you mention this, it reminds me that this was the real reason why we removed {{line}} support from the open command. It was actually what prompted the whole redesign of open and edit in the first place. 😅

If you set openCommand to code {{filename}}:{{line}}, then this works nicely for when you are in the staging panel, because it takes you to the right line. If you do this from the files view though, it jumps to line 1, which is annoying if the file was already open in VS Code and your cursor was likely close to the right place already. The only way to solve this would have been to have two command templates, one for open and one for openAtLine. This doesn't really fit the concept of the open command though, so we did that only for edit but not for open.

DusanLesan commented 1 year ago

If you do this from the files view though, it jumps to line 1, which is annoying if the file was already open in VS Code and your cursor was likely close to the right place already

Oh, you are right, resetting current focus would feel bad. I presume you could just omit the line part in defaults and leave the variable available to users like before? I would not mind any changes as long I can have two different commands opening two different editors at specified line.

stefanhaller commented 1 year ago

Yes, that's what I meant above. I don't have a super good feeling about this, it feels like abusing the open command for something it wasn't meant for. I'd prefer it if we could make the line available to custom commands so that you can use that instead. (I haven't looked at how hard that would be yet.)

DusanLesan commented 1 year ago

I would like to have line available for custom commands as well. So, if you plan to implement it, there's no need to adjust the designs of open command on my account. I am willing to wait for the addition of line in custom commands for however long it might take, since I believe it is a very useful feature to have. In the meantime, I can just make a wrapper that will be mapped to edit and decide what editor to use

jesseduffield commented 1 year ago

Something that we did consider (or maybe only I considered in my head) was allowing you to set multiple editors where if multiple are configured you get a menu popup asking you to select which one you want. Perhaps that's the best way to address this problem. It would require a change to the config structure though to allow an array rather than just the one editor config.

I'm also fine with adding '{{line}}' to custom commands but if it's a big use case to select the editor you want to open in, I think it's worth the effort to just support that properly.

stefanhaller commented 1 year ago

Yes, we considered this, but rejected it because we felt it's unjustified complexity for the few people who would use it. I still think that's right, although it's hard to tell because we don't conduct proper user research. 😄

Personally, even if I had the requirement of occasionally wanting to use a different editor, the price of having to type two keystrokes every time I want to edit would be too high for me. A custom command seems better to me for this, it's just one keystroke both for my main editor and for any additional ones I configure.

jesseduffield commented 1 year ago

Fair, although the hard part with the custom command approach is that you would need to implement one custom command per config value e.g. edit, editAtLine, etc.

stefanhaller commented 1 year ago

Wouldn't that just be different custom commands for the different contexts, but using the same key? Seems actually perfect to me.

jesseduffield commented 1 year ago

You could be right. Let's add that line to our custom command params and then see what the final product looks like