Support setting the limit of similarity for rename detection

[isti115]

Is your feature request related to a problem? Please describe.
I am in the middle of migrating a medium sized project to TypeScript from JavaScript, so I'm constantly renaming files from .js extensions to .ts while adding type annotations to them. In some cases this changes the files so much that git considers it as deletion and an addition instead of a rename, which makes reading the diff quite difficult. I find myself using this otherwise very convenient program less and less, and moving to the command line, where I can pass the --find-renames (or -M) flag to set the similarity threshold manually.

Describe the solution you'd like
A mechanism similar to the already existing whitespace display toggling in diffs, but for setting the similarity threshold required for two files to be considered the same. I don't think that this needs to be exposed on the UI like ctrl+w, it could be just a setting in the config file. (e.g. git.diff.renameThreshold)

(While we're at it, git.diffContextSize could become git.diff.contextSize and an option could be introduced for setting the display of whitespaces in diffs on startup, such as git.diff.showWhitespace.)

Describe alternatives you've considered
Creating a custom command that I can map to some key from lazygit itself, so I don't have to run a separate terminal with the git cli.

I have also tried looking for a gitconfig setting alongside status.renames, diff.renames and merge.renames, but couldn't find what I'd like. (The related *.renameLimit options affect the number of files considered, not the similarity threshold.)

---

ps. I am willing to towards this as a PR, but I would first like to get confirmation that it would be a welcome contribution.

[stefanhaller]

Yes, I'd say this would be a welcome improvement. The question is what the best UI for this is.

It feels to me like we are almost at a point where it would be nice to have a dialog with all sorts of diff options; a checkbox for ignoring whitespace, a text field for the context size, and a text field for the similarity threshold. Of course we are lacking the widgets necessary for that, so I guess I'm dreaming.

Meanwhile, I could imagine a UI either like for the context size (which can be increased/decreased with { and }), or a simple prompt that lets you type a value. Probably the latter, but in any event I do think it should be changeable at runtime.

It's also interesting that we have this discrepancy between ignoreWhitespace and contextSize. Too bad that we didn't notice this when discussing the ignoreWhitespace option recently. For context size, we have a config option, and commands to change them at runtime, but we don't write the value back to the config. For ignoreWhitespace we considered doing the same, but then decided to not have a config option and instead write the value to state.yml. I think we should consider removing the git.diffContextSize and doing the same for the context size.

And then, the similarity threshold should also follow the same pattern.

[isti115]

I have finally found some time to start working on this, and encountered another question to be discussed:
The --find-renames flag is useful for git status as well as git diff, but those have completely separate configuration objects in lazygit, which makes it inappropriate to categorize this either under diff.renameThreshold or status.renameThreshold, unless of course we'd like to have two separate parameters.

At first I'll make efforts towards having a working solution that can load from config and maybe adjust on-the-fly, and once that's implemented we can start figuring out how to converge the behavior of diffContextSize, ignoreWhitespace and renameThreshold. (I personally like the philosophy of having runtime adjustments be temporary and consider overwriting values in the user's config file a bit impolite, unless explicitly asked to do so with e.g. a "store config" command.)

[stefanhaller]

It's cool that you are working on this! I don't agree with your plan though: I don't think it makes sense to introduce a config option now, and then remove it again later.

Maybe I just wasn't clear enough on what my proposal is. I'm proposing to get rid of the git.diffContextSize config, and store it in state.yml instead, like we do for ignoreWhitespace (of course, this could be a different PR, it's independent from what you are doing). For your feature, I'd then do the same; no config option, start with a default value of 50%, let the user change it at runtime with a command that shows a prompt to type the value, and store that in state.yml.

Of course, it would be good to hear from @jesseduffield first if he has any objections.

[isti115]

The reason I wanted to go towards the config file route first was because it seemed easier to implement, but I also think that I get why you'd like these to be moved to state.yml instead. If I understand the philosophy behind the two files, config.yml should for example be checked into a dotfiles repository and shared between machines, while state.yml should be machine specific. (In which case .local/state/lazygit/state.yml [or $XDG_STATE_HOME if present] might be a better location for it. I'm inclined to fix config paths. 😀)

Anyway, I'd also like to read @jesseduffield's opinion on this!
(Personally I'd prefer all of these options to reside in the config and be temporarily modifiable interactively for the duration of only a single session.)

[stefanhaller]

Storing state.yml in XDG_STATE_HOME: you're right, and this is actually a problem for some people whose XDG_CONFIG_HOME lives on a read-only filesystem. See #2856. There's already work underway to change this, see #2936.

But my reason for suggesting to store the rename threshold in state.yml is not because it's machine specific, but because it's situation specific. You will want to change this on the fly (because you're looking at a diff where renames are not detected, so you want to increase it a bit). For this reason it doesn't make sense for it to be a config; you'd have to quit and relaunch lazygit for it to take effect, that's cumbersome. It's the same for the diff context: you change it on the fly because you need to, for the specific diff that you're looking at. Afterwards you might change it back to what it was, or you might actually leave it because you decide you like it better this way in general. Either way, saving it in state.yml is nice because you don't have to manually take care of entering the value that you just learned is your new preferred default value into config.yml manually.

[jesseduffield]

TL;DR This is tricky, I'm undecided.

So it sounds like the crux of the matter is that when there's an option that we want users to be able to change within lazygit, we need to decide between three approaches:
A) configure default with config.yml, update within lazygit (but don't persist)
B) configure both default (persisted in state.yml) and current value (not persisted) within lazygit
C) configure within lazygit and persist (in state.yml)

Whatever we decide on should apply to everything in the new widget so that there's no user confusion.

All of these options have pros/cons.

Option A/B are good when you really do want a default and don't want to bother resetting to the default manually. Option C is good because it's simple and has the minimum amount of friction, provided you're happy to open the next lazygit session with whatever you've got in the current session.

Comparing A and C, I feel better about A because a default value is a set-and-forget thing, and so far that's been handled in config.yml, so it's more consistent in that sense. I'll admit, it's also easier to implement.

So in my mind it's a choice between A and C

Existing use cases

Thinking about some existing use cases:

Changing diff context lines: I prefer having a configurable default for this, but it's not a strong preference. Typically if I'm changing context lines it's going from 3 to some large number so that I can see how a hunk is situated in the code. Although I'll then immediately shrink it back again in the same session, it's nice to know in the next session it'll be back to 3, but I only have experience with the existing behaviour: it probably makes no difference.

Ignoring whitespace: For this we don't have a default and that seems to be working fine. I did worry that people might forget they had whitespace being ignored, and so we show 'whitespace ignored' in the top-right of the main view. I don't know if anybody's been tripped up on that.

File rename percentage: I don't have this use case so don't have a strong opinion. I would have assumed it would be preferable to not have a default for this so that you can change it on the fly and have that persist.

I'm really in two minds about this: we're comparing the friction of having to go and set a default with the friction of just changing the value in-app to what you want when you want it. And I do consider the latter friction to be far less.

Comparing to other apps

• Github persists ignoring whitespace within a PR, but not across PRs. Lots of people want to be able to set a global default: https://github.com/orgs/community/discussions/5486. Nobody's asking to have the value persisted based on whatever was last selected in a given PR though for all I know they'd be happy with that.
• gitui has a widget for adjusting various options, all of which are persisted (no concept of defaults). Can't see anybody complaining about this in the issues

In conclusion: It's not clear to me which is the superior choice. I've made a twitter poll to see what others think.

Happy to just decide on something tomorrow to keep things moving, but also keen to get your thoughts on the above

[stefanhaller]

I don't really understand what option B is supposed to mean, so for me it's a choice between A and C. Currently, we have A for context size and C for ignoring whitespace.

It's interesting: you seem to argue for option A for context size with the same arguments that I used for ignoring whitespace, here. 😄 I now got used to the behavior of persisting it in state.yml, and I'm fine with it, although it did happen to me once that I accidentally left it on and got confused about that later. But I suppose that could also happen within one session, so it's not a strong reason.

And for context size there's no potential for confusion when you accidentally leave it at the wrong value (for annoyance, maybe, but not confusion).

In my opinion there are more good arguments for option C than against it, so my vote clearly goes to that for everything.

I've made a twitter poll to see what others think.

I don't know what twitter is, so I can't participate. Count my vote for C. 😄

For what it's worth, I started working on a PR that converts context size to option C, and it is a bit more work than I thought. I'll pause that now to wait for the poll result.

[jesseduffield]

I don't really understand what option B is supposed to mean

B is what @isti115 suggested: support setting both the current value and default from within lazygit.

At any rate, poll has come back in favour of C so let's go with that. That is, let's:

• support setting the value in lazygit
• store the result in state.yml
• use the value in state.yml when starting lazygit as opposed to using a configured default
• don't use config.yml

[stefanhaller]

Cool. An implementation question then: currently, many git commands methods (e.g. StashCommands.ShowStashEntryCmdObj) take the ignoreWhitespace flag as a bool parameter. They didn't have to take the context size as a parameter because they can get at it from self.UserConfig. Would you say it's appropriate to add AppState to common.Common so that everyone has easy access to these options? The other option is to create a DiffOptions struct containing the three options that we pass to each of these methods. Clients have to initialize it from AppState then, which is a lot of boilerplate.

[jesseduffield]

I'm happy with that

[stefanhaller]

OK. Here's a PR that moves the diff context size to state.yml: #2969. @isti115, adding the rename threshold right next to it should be easy if you build on top of it.

[isti115]

@stefanhaller Thank you for the example! I have finally found some time to work on this again, and submitted a PR: #3025

[isti115]

In the meantime I have been getting familiar with tig as a tool for exploring the history of a repo, since lazygit doesn't have blaming support, and saw that they have support for setting extra git flags through environmental variables. Using that I can simply set the similarity threshold in the following way:

TIG_DIFF_OPTS=-M1% tig

That might be another idea worth considering.
Also, they support colocating their configuration in the default git config file, which may or may not seem like a good idea depending on how you look at it.