-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Add character countdown to commit message input #36890
Conversation
There is a setting in "config.enableLongCommitWarning": "Whether long commit messages should be warned about", I don't know how the settings work in VS Code presently, but you might be able to work with that setting about whether to display the box. I would suggest adding an additional setting for the number to cut off rather than hardcoding 72… something like "config.longCommitWarningThreshold": "The number representing the maximum limit for an acceptable commit message", I don't have the build tools in front of me or anything yet. As an aside: I try to keep all my commits to <= 50 characters, since I think that's what the cutoff used to be in VS Code before the feature was dropped this year :) |
@scottclayton The warning feature was dropped when Git moved into an extension. I would imagine commit message limits are universal across SCMs anyway, within organisations or projects at least and not limited to the Git extension. As for settings, I can’t find an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to have this when Git wasn't an extension. Now that it is, this behaviour needs to go into API.
Would you like to continue the work and expose SourceControlInputBox.warningLength: number | undefined
?
@joaomoreno Forgive my ignorance, still new to the codebase. Is the API for exposing the setting from host to extension, extension to host, or both? As I argued above, I think users rarely need different length warnings for different SCM extensions. |
It's our extension API. It's not a user setting, it would be an API exposed to an extension. Git might use 80, while Perforce doesn't have a limit. |
@joaomoreno I’ve hard-coded Git’s warning length for now because I’m unsure where it should go. I would appreciate some guidance on localising strings as well. Do we have plural support? |
Any update on this? It's been a month since there was any activity |
@DuckyCrayfish It already works perfectly, and I occasionally rebase onto master while waiting for feedback. If you want to see this merged, 👍 the topmost comment to upvote! That’s how Microsoft ranks what goes into the next release. |
@jayjun Oh, wonderful, I got thrown off by the CI failure. Thanks for getting back to me, I'll give this a thumbs up. |
Hi guys ✋ sorry for jumping in so late. I've made a few changes:
Thanks! 🍻 |
Hi @joaomoreno did the |
@Rylon We had to change it to |
Unfortunately, the configuration variable |
@jlelong Great catch! I've updated the setting name and described the screenshot's behaviour: microsoft/vscode-docs#1387 Thanks! 🍻 |
I've also been struggling with this a bit, I was searching for "commit" and "length" in settings which didn't find anything because of this:
Maybe the comment could include a couple of useful keywords 😉 |
Please, @joaomoreno , any way to customize the characters warning limit? |
Thanks @joaomoreno |
@robsonsobral Sure, would you like to submit a PR? |
There's an issue specifically for that: #18807. |
@joaomoreno , I'm not sure on how to proceed, but I gonna try. |
I really like that this feature has made it in but I would like it to actually follow the very common pattern of having a 50 character summary line, and then an empty line before allowing a 72 character body. It would be really nice if that would work somehow or could be configured in a way, although I’m not sure if that would not conflict with that API setting mentioned above since that only applies globally, and would not allow for line-based validation. |
@poke I would definitely prefer the way you described to work to be implemented, but maybe doing what was described at #18807 (comment) could be a simpler solution, as it would attend all public. It would be nicer though if you could add three settings to what we already have today, those being |
See #21144.
I miss warnings against long commit messages. I like Atom’s way of showing a persistent character countdown. It’s subtle and easy to ignore.
Knocked something up quickly using
InputBox
’s built-in validation box.Haven’t put much thought into the UI. Happy to hear more ideas!