Add support for setting the similarity threshold for detecting renames (closes #2904)

[isti115]

• PR Description
  Implement the feature as discussed in Support setting the limit of similarity for rename detection #2904.

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• Docs (specifically docs/Config.md) have been updated if necessary
• You've read through your own file changes for silly mistakes etc

---

Edit: Previously I have left the "Text is internationalised" checkbox empty, as I thought that it would also have to include translations, but as I understand currently, that would be "localization" instead, so I ticked it.

[stefanhaller]

Great work so far, this looks awesome.

My main concern is about the UX: I find it cumbersome to change the threshold in 1% increments (especially when I want to change it back to 50% when I'm done). Could it be acceptable to change it in 5% steps? This would make it much easier, I think.

Apart from that, there are both unit tests and integration tests failing. I haven't looked into why, let me know if you need help figuring this out.

[stefanhaller]

I think it's intentional that we turn off rename detection here, because it makes it easier to create custom patches from added or deleted files.

[stefanhaller]

@isti115 I haven't heard back from you after giving feedback, and it's been quite a while. Do you intend to keep working on this?

[isti115]

@stefanhaller Yes, I still have it on my (ever growing) todo list. I really hope to find time to fix this up soon, but life has been busy lately. Still, each week I hope that I might just randomly find an hour of free time to work on this. 🙃

[isti115]

@stefanhaller I have finally managed to sit down and hopefully get this on track. 🙂
Can you please take a look at the current version? As far as I can tell the only concern I didn't address is the step size of the threshold adjustment. In my opinion adjusting bigger ranges with one percent granularity is not really an issue, since the key can be held down to repeat the action until the desired value is reached. Of course if you insist on changing it to steps of 5%, I'll implement it that way.

[stefanhaller]

In my opinion adjusting bigger ranges with one percent granularity is not really an issue, since the key can be held down to repeat the action until the desired value is reached.

This solves it for people who use a very fast key repeat rate (I'm one of those), but for people with a slower repeat rate it's still cumbersome. Also, my main concern was about setting it back to 50% when you're done, which is cumbersome with a 1% granularity.

That said, while I do feel that using a 5% granularity could be an improvement for both, I'm not sure it's the best solution. Maybe there's a completely different option that we are both missing; would be interested in @jesseduffield's take on this. So no, I definitely don't insist.

Meanwhile, there are still unit tests failing. You can use go test ./... -short (or make unit-test) to run them locally, or use VS Code's test browser (that's what I usually do).

[isti115]

@stefanhaller Sorry for needing so much babysitting with this PR, this is at least partly due to the fact that I was trying to get this done quickly while having very little previous Go experience. 🙃

I really hope that I won't be taking away much of your attention / time now! (The remaining failing checks [missing label and project token] seem to be ones that are not under my control.)

I'd also be interested in reading the opinion / ideas of others regarding the step size discussion!

[mark2185]

I wonder how many people would actually use 93% instead of 90% or 95%.

I've never used it before so I'm curious to hear how precise does it have to be for actual users.

Also just a nitpick (that I don't expect to be addressed) - holding down the ( or ) doesn't display the increase/decrease message in real time :(

(it makes sense, I just thought it would go brrrrrrrr)

[isti115]

@mark2185 Hmm, interestingly enough it does "go brrr" for me. 😀

[mark2185]

Huh, that's odd, it's laggy on my side, tried two different terminal emulators.

In any case, if it works for you, consider the matter solved, then!

[isti115]

Sorry to bump, but in order to prevent the PR from going stale I'd like to ask, @jesseduffield, if you have any further input based on the discussion above? I wonder whether the question of step size is significant enough to be blocking the merge.

[jesseduffield]

Steps of 5% sounds good to me

[isti115]

Sorry for disappearing again, I've adjusted the step size to 5%.

[isti115]

Sorry for the bump again, @jesseduffield and @stefanhaller, by no means do I feel entitled to quick responses after my several month long hiatuses, but just in case you didn't get a notification of my previous post I thought I'd also tag you to prevent this PR from getting lost indefinitely. 🙂

[jesseduffield]

@isti115 I've just had a look through the code: This is great work, I'm happy to merge it. You just need to fix that lint issue (looks like a code formatting issue) and rebase onto master. There's a conflict in the test_list.go file but that'll be easy to resolve: just accept both changes and then re-run go generate ./...

[jesseduffield]

I've taken the liberty of making those changes myself as I'm itching to get this release out today!

[isti115]

Wow, it's really weird how my latest commit had those bunch of misalignments, since I had automatic formatting enabled during development. I guess that they must've been introduced during my the latest rebase, maybe I had --ignore-whitespace turned on. Thanks for fixing things up, I'm looking forward to being able to easily access this feature anywhere, not only on my own builds! 🙂