lazygit
lazygit copied to clipboard
Added row length in commit description title
- PR Description
Simple but effective implementation which allows the users to see the amount of characters for each row. PR is stemming from discussions on how to encourage/enforce git best practices here and here.
I think this can be a good start towards providing further aid for the users to break their rows at a reasonable limits.
- Please check if the PR fulfills these requirements
- [x] Cheatsheets are up-to-date (run
go generate ./...
) - [x] Code has been formatted (see here)
- [ ] Tests have been added/updated (see here for the integration test guide)
- [x] Text is internationalised (see here)
- [x] Docs (specifically
docs/Config.md
) have been updated if necessary - [x] You've read through your own file changes for silly mistakes etc
Nice. I like the simplicity of this approach; on the other hand, I have concerns whether this is useful enough. A few observations:
- showing the length of the line is only useful some of the time. It helps for when you are typing at the end of the line to determine when you need to hit enter, but that's the only case where it's useful. For example, if you add a few words in the middle of the line, you can see that the line is now too long, but you then need to figure out where to break it so that it fits again, and the display doesn't help with this. You have to use trial and error until you found the right spot. I wonder if it's more useful to show the current cursor column rather than the line length; if you are typing at the end of the line, both are the same, but if you try to find the best spot where to break a long line, the cursor position helps more. However, the cursor position doesn't help see at a glance whether a line has become too long when typing in the middle of it; you first have to jump to the end of the line to find out. Plus, showing the current position makes it inconsistent with the subject field, and for that one you do want to see the line length. It's a bit of a mess to get right, I feel.
- a minor cosmetic thing: it bothers me how the subtitle field wiggles once the length gets longer than 9, or when you use up/down arrow to move through lines that alternate between being longer or shorter than 9. Maybe you should pad one-digit numbers with a space on the left to avoid that.
I don't know, all in all I feel I want a better way to solve this, one that requires less manual rewrapping work. I just had a look at Thunderbird's message composition window again, and I feel that it solves this problem in a very elegant way. It just rewraps paragraphs automatically as I type, this is exactly what I want. Here's a video:
https://github.com/jesseduffield/lazygit/assets/1225667/57d55492-fb3e-482a-b4af-af4df969b80a
Yes, I know that in the past I argued against automatic rewrapping, but that was in response to a rewrap command that you invoke. I feel this is dangerous as long as we don't have undo in our editor. However, the fully automatic as-you-type rewrapping seems really perfect to me, and I can't imagine a lot of situations where it would do something I don't want. (One such case might be if I want to insert a long http link and I don't want it to be broken; but I can always type <c-o>
and do it in my editor).
Now, I haven't thought much about how this could be implemented, or how hard that would be. I feel we'd need a change to gocui, but I'm not sure. In any case we'd need a way to distinguish hard line breaks from "soft" ones, somehow.
Anybody up for giving this a try?
I think the changes you're suggesting would need to be in gocui, yes. It should be fairly straightforward to do, a fun challenge to make it efficient enough even on long messages. I think the biggest issue would be how to signal to the users what it is and how to configure it. If it's on by default, it would be quite intrusive to the users (but perhaps a welcomed change? I would think so) and if it's not, it would very difficult to know it's even a feature. I'll give an implementation a go in a separate PR, I don't think that the changes are mutually exclusive.
This PR's indicator is intended to be a middleground. For the ones who doesn't care much about row length: it won't be in the way. And for the ones who do: they can see how long each row is, and then themselves decide where to break it, or perhaps decide to edit it in an editor. It's the agnosticism which is the strength though: anyone can decide their own limts. I added fixed padding, up to 3 digits.
I think the biggest issue would be how to signal to the users what it is and how to configure it. If it's on by default, it would be quite intrusive to the users
I haven't seen this as a problem so far. I have been including #3173 in custom builds that I distribute to colleagues at work for a while now, and observed some of them use it for the first time. There's usually a brief moment of surprise when they see the auto-wrapping for the first time, and from then on it's totally intuitive.
I don't think that the changes are mutually exclusive.
I do; once we have auto-wrapping (and I really want #3173 to get merged), we don't need the row length indication any more, and it's just unnecessary visual noise. I'd vote for closing this PR in that case.
Sounds good to me! Feel free to close this when #3173 gets merged.