Skip to content

Conversation

@stefanhaller
Copy link
Collaborator

Addresses #4900.

@stefanhaller stefanhaller added the enhancement New feature or request label Sep 22, 2025
@codacy-production
Copy link

codacy-production bot commented Sep 22, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for cb2b5c31
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (cb2b5c3) Report Missing Report Missing Report Missing
Head commit (b599dfc) 57739 50190 86.93%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#4912) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

  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.

@pbstrein
Copy link

Thanks for the fast response!

This works if the footnotes are separated by a newline
Example:

[1]: https://www.example.com
[2]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue 

[3]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue

However, when you try to put them all together without newline breaks, the same issue occurs
Example:

[1]: https://www.example.com
[2]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue 
[3]: 
https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue

@stefanhaller
Copy link
Collaborator Author

In your second example, it seems that you were trying to separate the 2nd and 3rd footnote by a space rather than a newline, and I'm not sure why you are expecting this to work. I only consider footnote symbols at the start of a (hard-wrapped) line, so your [3]: after the space is in the middle of a line and doesn't get the special treatment.

(It would be easy to change the logic so that it also considers footnote symbols at the beginning of soft-wrapped lines, but honestly I don't see why it should, and it seems conceptually wrong to me.)

In your first example, you are using two newlines after the 2nd footnote; that's why you get the blank line. If you use a single newline (like you do after the first footnote), you get the desired result:

[1]: https://www.example.com
[2]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue
[3]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue

@pbstrein
Copy link

Lets try a different example.

When I have the following in my commit message created in a different editor (VIM in my case)

[1]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue
[2]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue
[3]: https://www.example.com
[4]: https://www.example.com

When I edit the commit in lazygit (press r), it turns into this:

[1]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue
[2]:
https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue
[3]: https://www.example.com
[4]: https://www.example.com

@stefanhaller
Copy link
Collaborator Author

@pbstrein Ah interesting. This is not so much a bug in the soft-wrapping algorithm, but a deficiency of the soft-unwrapping that we do when rewording.

To explain: when you reword a commit, we run an algorithm that tries to remove as many hard line breaks as it can. This is important so that when you then add or delete a word at the beginning of a paragraph, it nicely reflows the text as if you had initially typed it. What this algorithm does is that it looks at every hard line break in the text, tries to replace it with a space, and checks whether the text up to that point still wraps the same way. If it does, it keeps the space instead of the newline.

Which means that in your example the algorithm replaced the hard line break at the end of the [1]: line with a space, but because I insisted that the footnote handling should only occur on hard line starts but not soft line starts, this causes the line to be broken after the [1]:.

The ideal fix would be to make the unwrapping algorithm smarter so that it doesn't replace the \n with a space in this case. I looked into this a bit this morning, and it made the code very complicated. The other way to solve it is to also do the footnote handling at soft line starts; this is a much simpler fix, so I went with that for now.

I pushed a commit in case you want to test it again.

@pbstrein
Copy link

pbstrein commented Oct 2, 2025

I tested that with a few commits over the last day, and works! Thanks!

What's the next step for getting this merged? I did some review of the code and lgtm, but I'm unfamiliar with the codebase, so I don't have the full context of where this fits / alternatives to accomplish the same goal.

@stefanhaller stefanhaller changed the title Don't break line after footnote symbol Don't break line after footnote symbol in commit messages Oct 2, 2025
@stefanhaller
Copy link
Collaborator Author

What's the next step for getting this merged?

Doing this as a proper PR in gocui, which I just did (see jesseduffield/gocui#85), and then bumping the gocui dependency here, which I will do right now.

Thanks for testing!

@stefanhaller stefanhaller force-pushed the avoid-line-breaks-in-footnotes branch from b599dfc to f7609f7 Compare October 2, 2025 15:26
@stefanhaller stefanhaller marked this pull request as ready for review October 2, 2025 15:27
@stefanhaller stefanhaller enabled auto-merge October 2, 2025 15:27
The main change here is jesseduffield/gocui#85, which
avoids breaking lines after footnote symbols in commit messages (e.g. [1]).

[1]: https://www.example.com/this-is-a-really-long-url-that-lazy-git-automatically-wraps-and-is-an-issue
@codacy-production
Copy link

codacy-production bot commented Oct 2, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for cb2b5c31
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (cb2b5c3) Report Missing Report Missing Report Missing
Head commit (ef92e30) 57739 50195 86.93%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#4912) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

  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.

@stefanhaller stefanhaller force-pushed the avoid-line-breaks-in-footnotes branch from f7609f7 to ef92e30 Compare October 2, 2025 15:29
@stefanhaller stefanhaller merged commit 05b6588 into master Oct 2, 2025
11 checks passed
@stefanhaller stefanhaller deleted the avoid-line-breaks-in-footnotes branch October 2, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants