Skip to content

Allow multiline commit messgages #1333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

heiskane
Copy link
Contributor

@heiskane heiskane commented Sep 14, 2022

This Pull Request fixes/closes #1171

It changes the following:

  • Allow using enter to create a newline in commit message box
  • Up/Down arrow changes the line the cursor is on
  • Keybind to confirm commit message had to be changed to ctrl + s
  • Works with multibyte/special characters
  • Fixes incorrect character count shown in commit message box when multibyte characters are used

I followed the checklist:

  • I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I added an appropriate item to the changelog

@heiskane heiskane marked this pull request as draft September 14, 2022 20:09
@heiskane
Copy link
Contributor Author

Just realized I still have to deal with non-ascii characters so I'm changing this to draft.

@extrawurst
Copy link
Collaborator

@heiskane thanks for tackling this!

@heiskane
Copy link
Contributor Author

heiskane commented Sep 25, 2022

@extrawurst
No problem although i am having some trouble with non-ascii characters. To clarify the multiline commit messages are in fact working already but the issues with the special characters happen when trying to use up and down arrows to move cursor up/down.

@extrawurst
Copy link
Collaborator

Yeah the multibyte chars need to work though :)

@heiskane heiskane marked this pull request as ready for review October 2, 2022 18:58
@heiskane
Copy link
Contributor Author

heiskane commented Oct 2, 2022

@extrawurst I'm still pretty new to contributing on GitHub and not sure if you get pinged when i switch my pull request back to open so pinging just to be sure.

@extrawurst
Copy link
Collaborator

can you rebase and fix the merge conflicts please?

@heiskane
Copy link
Contributor Author

@extrawurst Done 👍

@extrawurst
Copy link
Collaborator

Screenshot 2022-10-20 at 16 43 43

probably best to squash yourself to figure out what's the problem here

@heiskane heiskane force-pushed the allow-multiline-commit-messgages branch from 2b71f35 to 9a55aa5 Compare October 20, 2022 14:56
@heiskane
Copy link
Contributor Author

heiskane commented Oct 20, 2022

@extrawurst Okay now i think its all correct.
ps. I was just being i bit dumb lol

@heiskane heiskane marked this pull request as draft October 22, 2022 12:52
@heiskane
Copy link
Contributor Author

Just realized that the existing MultiLine input type does not really mean what i thought it means so this does actually break the input boxes in other places. Not sure how i didnt notice but either way I'm changing this back to draft and ill try to fix it asap 👍

@heiskane heiskane marked this pull request as ready for review October 22, 2022 13:23
@heiskane heiskane force-pushed the allow-multiline-commit-messgages branch from a3991d3 to 92c10c3 Compare October 22, 2022 13:44
@heiskane
Copy link
Contributor Author

heiskane commented Oct 22, 2022

Sorry for almost messing up there 😬. I thought that the MultiLine type indicated that multiline messages are allowed but that wasn't quite it. I have changed other input boxes correctly to SingleLine and increased the singleline input box to make sure that stuff like longer branch names will fit into it (62 characters should be enough to fit right?). Maybe singleline input box could grow in size as needed in the future? I double checked that all input boxes are worked as they should and re-ran tests.

Also seems like i have a thing or two to learn about rebasing but as far as i can tell it should be all good to merge 👍

@extrawurst
Copy link
Collaborator

Screenshot 2022-10-24 at 16 36 27

please squash all your changes together and rebase on top of current master

@heiskane heiskane force-pushed the allow-multiline-commit-messgages branch from e19fc99 to 6e76ec6 Compare October 24, 2022 16:14
@heiskane heiskane force-pushed the allow-multiline-commit-messgages branch from 6e76ec6 to 34d9ae1 Compare October 24, 2022 16:20
@heiskane
Copy link
Contributor Author

Squashed

@extrawurst
Copy link
Collaborator

extrawurst commented Oct 26, 2022

I tested it again and it does not keep the cursor in view aka it is not scrolling to the line we are editing it only ever keeps the first x lines visible.

let me know in case you cannot reproduce this - you just need to press enter a few times.

putting it in draft till fixed

@extrawurst extrawurst marked this pull request as draft October 26, 2022 12:44
@heiskane
Copy link
Contributor Author

I tested it again and it does not keep the cursor in view aka it is not scrolling to the line we are editing it only ever keeps the first x lines visible.

let me know in case you cannot reproduce this - you just need to press enter a few times.

putting it in draft till fixed

I just haven't implemented the scroll yet. I kinda felt like that would be worth another issue but I'll try add it to this PR when I have time 👍

@extrawurst
Copy link
Collaborator

you are right that it is worth doing a separate smaller PR for. but lets reverse the order and have that separate PR first, because as it is right now the scrolling would already be useful to have in the current state of gitui. can you create a separate PR for that based on current master? this way we do not blow this one up more and still get a nice value added once the first (hopefully smaller) PR is merged. wdyt?

@heiskane
Copy link
Contributor Author

Sure that works for me 👍

@extrawurst
Copy link
Collaborator

superceded by #1963

@extrawurst extrawurst closed this Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create multi-line commit messages
2 participants