Exclude commit trailers from line wrapping #97
Conversation
This brings in jesseduffield/gocui#97 ("Exclude commit trailers from line wrapping").
stefanhaller
force-pushed
the
dont-wrap-commit-trailers
branch
from
c100b6d to
4d58a88
Compare
sogladev
left a comment
sogladev
left a comment
There was a problem hiding this comment.
I forgot Strings.builder was a thing when writing. This is much nicer.
I tested the changes in jesseduffield/lazygit#5230, and it works as expected
Closing my PR (jesseduffield/lazygit#5217) as it's no longer needed
| return | ||
| } | ||
|
|
||
| if len(chr) != 1 { |
There was a problem hiding this comment.
This is minor, but footnotes are also all ASCII, and the same optimization can be used to skip some regex checks
There was a problem hiding this comment.
Interesting, but that would actually make it worse. I added a bit of benchmark data (587285e) for this, and adding the check to the footnote matcher increases the benchmark time by a couple of microseconds.
I need to keep the check for the trailer matcher though, because it's needed for correctness, since I access the bytes of the string a few lines below, so I first need to make sure it's really one byte.
stefanhaller
force-pushed
the
dont-wrap-commit-trailers
branch
from
4d58a88 to
30a4af5
Compare
|
@sogladev Thanks for the review! Merging. |
This is calling contentToCells for every key press. As we are going to make changes to that function in this branch, we want to make sure that we don't cause a terrible performance regression with our changes. On my machine, one benchmark iteration takes 150μs, which is already a lot less than I had expected.
Line-wrapping a commit trailer that contains a very long email address results in a broken trailer (trailers do support being split into several lines, but this requires continuation lines to start with a space, like in RFC-822 message headers). To avoid this, simply never wrap commit trailers. It's a bit questionable that we hard-code this behavior in TextArea, which is meant to be a general-purpose widget; but we know we only use it in lazygit's commit message panel, so don't bother making this configurable for now. Benchmark time increases from 150μs to 165μs on my machine, which is still more than acceptable.
stefanhaller
force-pushed
the
dont-wrap-commit-trailers
branch
from
30a4af5 to
8029be2
Compare
This brings in jesseduffield/gocui#97 ("Exclude commit trailers from line wrapping").
Bump gocui, which brings in jesseduffield/gocui#97. #Fixes #5216.
2 participants
Line-wrapping a commit trailer that contains a very long email address results
in a broken trailer (trailers do support being split into several lines, but
this requires continuation lines to start with a space, like in RFC-822 message
headers). To avoid this, simply never wrap commit trailers.
It's a bit questionable that we hard-code this behavior in TextArea, which is
meant to be a general-purpose widget; but we know we only use it in lazygit's
commit message panel, so don't bother making this configurable for now.
Fixes jesseduffield/lazygit#5216.