lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Auto-wrap commit message while typing

Open stefanhaller opened this issue 2 years ago • 11 comments

  • PR Description

Add new config settings git.commit.autoWrapCommitMessage (default true) and git.commit.autoWrapWidth (default 72), which allow automatic as-you-type wrapping of the commit message body to the specified width.

There are occasional situations where this wrapping is in the way, for example when you need to have longer lines in the message for some reason (perhaps because you have a very wide ASCII art picture or table), and you'll have to resort to switching to the editor in that case. However, in my experience these cases are quite rare.

  • 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)
  • [x] Docs (specifically docs/Config.md) have been updated if necessary
  • [x] You've read through your own file changes for silly mistakes etc

stefanhaller avatar Dec 20 '23 14:12 stefanhaller

This is still a draft for two reasons:

  • it needs a PR in gocui first, if we're happy with the way I implemented it
  • it doesn't work well for rewording commits. In that case, the existing commit message already contains hard line breaks, so adding words at the beginning of an existing paragraph will not work as expected. For now you'll probably resort to switching to the editor for rewords, most of the time. I have some ideas for how to remove the hard line breaks of a reworded message, but it's a bit tricky; we have to be careful not to mess up tables or bullet point lists.

stefanhaller avatar Dec 20 '23 14:12 stefanhaller

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
:white_check_mark: +0.03% (target: -2.00%) :white_check_mark: 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (af56065dd69885dc394a78387ae07feb53162baf) 48350 40513 83.79%
Head commit (d82a09f4f2f64c40f79b3f80835edc867daf9be7) 48433 (+83) 40596 (+83) 83.82% (+0.03%)

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 (#3173) 95 95 100.00%

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

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

codacy-production[bot] avatar Dec 20 '23 14:12 codacy-production[bot]

  • it doesn't work well for rewording commits. In that case, the existing commit message already contains hard line breaks, so adding words at the beginning of an existing paragraph will not work as expected.

I added a commit to improve this considerably. See the commit message of the last commit for details.

stefanhaller avatar Dec 20 '23 18:12 stefanhaller

I've tested this out locally and I'm surprised it works so well, especially given that when I hard wrap in vscode I need to manually-rewrap myself if I insert into the middle of a paragraph. I haven't had the time yet to look closely at the code though

jesseduffield avatar Jan 26 '24 11:01 jesseduffield

Discussed: make two configs

    autoWrapCommitMessage: true
    autoWrapWidth: 72

Don't insert line breaks into single long words (e.g. URIs).

stefanhaller avatar Feb 16 '24 09:02 stefanhaller

@jwhitley @mark2185 @afhoffman @AzraelSec @karimkhaleel

I'm writing this to encourage people to get more involved with the lazygit project beyond just contributing PRs (only if they feel like it of course).

We have a situation where @jesseduffield (the maintainer) doesn't have enough time to do thorough reviews of all PRs that are submitted. I'm trying to step in as much as I can, to help keep things moving, but I don't always have enough time either, and there's nobody who does this for my PRs.

So I'm tagging a few people here who have recently contributed to lazygit to see if they might be up for trying to review PRs, e.g. this one. I fully realize that you probably wouldn't feel comfortable approving PRs, but that's not necessarily what this is about. We just need another pair of eyes on code that is being merged.

Here are some things that people can do even if they don't have a good overview of the code base or architecture yet:

  1. Test the feature. You don't need to look at the code at all for this; just look at the PR description, and test if the feature works the way you'd expect it to.
  2. See if you can understand the implementation. If not, maybe some variable or function names can be improved to make it clearer.
  3. Look at the commit messages to see if they adequately describe what's being changed.
  4. Look at the user-facing texts to see if the wording can be improved.
  5. See if there are gaps in the test coverage (if there are tests at all).

You'll see how your knowledge of the code base increases quickly if you read PRs that touch various parts of it. And when we all do this for a while, maybe we can get to a state where it feels more like a team working on a project, rather than a bunch of individuals working separately without looking at each other's work. (Maybe that's a dream, but it would be cool.)

stefanhaller avatar Feb 23 '24 16:02 stefanhaller

I fully realize that you probably wouldn't feel comfortable approving PRs, but that's not necessarily what this is about. We just need another pair of eyes on code that is being merged.

Yes, I don't think you want me approving PRs for now. I shouldn't be deciding what does and doesn't "belong" in lazygit, but I'd be very happy to spend a little more time checking if things work and are understandable at least.

You'll see how your knowledge of the code base increases quickly if you read PRs that touch various parts of it. And when we all do this for a while, maybe we can get to a state where it feels more like a team working on a project, rather than a bunch of individuals working separately without looking at each other's work. (Maybe that's a dream, but it would be cool.)

I really appreciate how welcoming you all have been. I have been enjoying the opportunity to contribute a little back to a tool that I use and enjoy so much. I would absolutely be up for spending a little more time looking at other pr's!

afhoffman avatar Feb 23 '24 17:02 afhoffman

Thanks for commenting, @stefanhaller! I'm always happy to contribute to this fantastic project (already have a couple of issues to work on atm). I'm also up for looking at the PRs if that makes your life easier (I could start with this one if that works for you). I'll keep an eye on the new contributions, but let me know if you have specific PRs you want me to review, or if works I could help you with 👍🏻

AzraelSec avatar Feb 23 '24 17:02 AzraelSec

@AzraelSec That's awesome. You don't have to ask for permission, just go ahead and post your thoughts on any PR. It also doesn't hurt if multiple people do it on a single PR, the more input the better. (I'll say "stop" if it gets out of hand, but I'm not worried about that for now. 😄)

Some PRs may be better suited for beginners than others. If most changes are local to one area of the code, it will be easier of course. Here's another one where this is the case: #3338.

And here's a super easy one that may be a good start: #3336.

stefanhaller avatar Feb 23 '24 18:02 stefanhaller

@stefanhaller I'm in, and thank you for the invitation. I've already been quietly surfing the "all activity" notifications firehose for lazygit for awhile. First, however, I need to get on my two outstanding PRs, esp. since one is on the release milestone. Some IRL commitments, just now past, held those up.

jwhitley avatar Feb 23 '24 22:02 jwhitley

I'll be happy to help. Thanks for the callout!

karimkhaleel avatar Feb 23 '24 23:02 karimkhaleel

Glad you guys are up for the challenge! Hugely appreciated :)

jesseduffield avatar Mar 02 '24 07:03 jesseduffield

I've used this a bit myself now and it's rock solid. I'm happy for this to be merged. Great work.

One thing is that I was noticing some flickering of the cursor as I typed the commit message but that's now gone away so I'm not sure what's changed but if it comes back we'll need to look into it.

jesseduffield avatar Mar 09 '24 03:03 jesseduffield

One thing is that I was noticing some flickering of the cursor as I typed the commit message

I also noticed some flickering of the cursor recently, but not only when typing in the commit message panel, but also in other prompts. And it also happens on master for me. I attributed it to changes in iTerm (without checking though), as I'm using the beta build and get an update every few days, which occasionally breaks things. I haven't checked other terminals yet.

stefanhaller avatar Mar 09 '24 08:03 stefanhaller

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
:white_check_mark: +0.02% (target: -2.00%) :white_check_mark: 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (dc9ee186f4d56f5a971012063ae27d2dd3ce276a) 48689 40883 83.97%
Head commit (d1f8c450995ace6b1b0e352ba10dcc0653da4030) 48772 (+83) 40964 (+81) 83.99% (+0.02%)

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 (#3173) 95 95 100.00%

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

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

codacy-production[bot] avatar Mar 09 '24 09:03 codacy-production[bot]