Improve "Find base commit for fixup" command when there are changes for master commits

[stefanhaller]

• PR Description

If exactly one candidate from inside the current branch is found, we return that one even if there are also hunks belonging to master commits; we disregard those in this case.

• Please check if the PR fulfills these requirements

• [x] Cheatsheets are up-to-date (run go generate ./...)

• [x] Code has been formatted (see here)

• [x] Tests have been added/updated (see here for the integration test guide)

• [ ] Text is internationalised (see here)

• [ ] Docs have been updated if necessary

• [x] You've read through your own file changes for silly mistakes etc

[stefanhaller]

@jesseduffield I have run into this situation enough times recently that I'm convinced now it's a good rule.

Two questions that I'm not totally sure about though:

• I don't put up a warning, like you suggested on Discord. In the cases that I encountered, the situation was so clear that I didn't think a warning is necessary. Happy to add one though, if you prefer. However, it's a bit unclear how to handle the case we also have a warning that there are hunks with only added lines; do we show them one after the other, or do we show a single warning containing both texts?
• The rule is in effect no matter whether we are blaming deleted lines, or the context of added lines. I wasn't sure that's good; personally I have only encountered the problem with deleted lines, so I was considering having the rule be in effect only in that case, and still throw an error in the other case.

[codacy-production[bot]]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 6cb2ac6fcce09830cd426e49a2c5f6b79db31a32[^1]	:white_check_mark: 82.28%

Coverage variation details

| | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (6cb2ac6fcce09830cd426e49a2c5f6b79db31a32) | Report Missing | Report Missing | Report Missing | | | Head commit (7780f1264ac34e32b5a52bed667feaa0a67fa8d2) | 52007 | 44966 | 86.46% | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `

Diff coverage details

| | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#3645) | 79 | 65 | **82.28%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences

---

_{:rocket: Don’t miss a bit, follow what’s new on Codacy.}

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more} [^1]: Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

[jesseduffield]

In response to your questions:

I don't put up a warning, like you suggested on Discord. In the cases that I encountered, the situation was so clear that I didn't think a warning is necessary. Happy to add one though, if you prefer. However, it's a bit unclear how to handle the case we also have a warning that there are hunks with only added lines; do we show them one after the other, or do we show a single warning containing both texts?

Happy to not have a warning! I only suggested a warning initially to sweeten the deal of my initial proposal :)

The rule is in effect no matter whether we are blaming deleted lines, or the context of added lines. I wasn't sure that's good; personally I have only encountered the problem with deleted lines, so I was considering having the rule be in effect only in that case, and still throw an error in the other case.

My vote is to have the rule in effect whether it's deleted or context lines. I'd rather go from there to tightening it if there are problems than the other way around.

[stefanhaller]

Happy to not have a warning! I only suggested a warning initially to sweeten the deal of my initial proposal :)

Ha, I suspected that. 😄 Glad that we agree on this one.

My vote is to have the rule in effect whether it's deleted or context lines. I'd rather go from there to tightening it if there are problems than the other way around.

Works for me.