Git input validation got broken in 1.85.0

[chrisant996]

Type: Bug

The git commit message input validation coloring and messages no longer show up. This was working a couple days ago, and stopped working when the editor auto-upgraded to v1.85.0.

Repro steps:

1. Launch VSCode in a git repo.
   • with or without extensions enabled; it repros either way.
2. Click Source Control in the Activity Bar.
3. Type a long commit message.

Expected:

• In the first line, characters past the git.inputValidationSubjectLength length should be highlighted in red, as they used to be.
• In subsequent lines, if the current line is longer than git.inputValidationLength then a message should be shown in the Message box indicating the number of extra characters past the validation length.

Actual:

• No highlighting, no overflow message.

It's like the git input validation feature has been completely removed. I didn't see anything in the release notes about that, and the settings still exist, so it seems like an unintentional regression.

My git input validation length settings are the following:

"git.inputValidationLength": 72,
"git.inputValidationSubjectLength": 50,

VS Code version: Code 1.85.0 (af28b32, 2023-12-06T20:48:09.019Z)
OS version: Windows_NT x64 10.0.19045
Modes:

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-10870H CPU @ 2.20GHz (16 x 2208)
GPU Status	2d_canvas: enabled canvas_oop_rasterization: enabled_on direct_rendering_display_compositor: disabled_off_ok gpu_compositing: enabled multiple_raster_threads: enabled_on opengl: enabled_on rasterization: enabled raw_draw: disabled_off_ok video_decode: enabled video_encode: enabled vulkan: disabled_off webgl: enabled webgl2: enabled webgpu: enabled
Load (avg)	undefined
Memory (System)	31.77GB (19.22GB free)
Process Argv	. --crash-reporter-id e22f69dd-b5d3-447a-b7b6-02eceab1bad6
Screen Reader	no
VM	0%

Extensions: none

A/B Experiments

vsliv368cf:30146710
vsreu685:30147344
python383:30185418
vspor879:30202332
vspor708:30202333
vspor363:30204092
vswsl492:30256859
vslsvsres303:30308271
vserr242:30382549
pythontb:30283811
vsjup518:30340749
pythonptprofiler:30281270
vshan820:30294714
vstes263:30335439
vscod805cf:30301675
binariesv615:30325510
bridge0708:30335490
bridge0723:30353136
vsaa593cf:30376535
pythonvs932:30410667
py29gd2263:30899288
vsclangdc:30486549
c4g48928:30535728
dsvsc012:30540252
azure-dev_surveyone:30548225
3biah626:30602489
f6dab269:30613381
2i9eh265:30646982
showlangstatbar:30737416
fixshowwlkth:30771522
showindicator:30805244
pythongtdpath:30769146
i26e3531:30792625
welcomedialogc:30910334
pythonnosmt12:30797651
pythonidxpt:30866567
pythonnoceb:30805159
asynctok:30898717
dsvsc013:30795093
dsvsc014:30804076
dsvsc015:30845448
pythontestfixt:30902429
pyreplss1:30897532
pythonmypyd1:30879173
pythoncet0:30885854
h48ei257:30885898
pythontbext0:30879054
dsvsc016:30899300
dsvsc017:30899301
dsvsc018:30899302
aa_t_chat:30882232
dsvsc019:30917259

[abhijit-chikane]

Observed the same.

[gjsjohnmurray]

Does #185606 (comment) help?

@lszomoru changed defaults are often controversial and if this didn't get called out prominently in release notes it's even more likely to catch people out.

[chrisant996]

Does #185606 (comment) help?

@lszomoru changed defaults are often controversial and if this didn't get called out prominently in release notes it's even more likely to catch people out.

Oh interesting. Yeah, the changing of that default could have been handled/communicated a little better probably.

Some feedback, then:

1. Ideally there should be UI right in the commit message box to allow turning validation on/off. Otherwise changing the default is disruptive and hard to find how to re-enable it.
2. Searching in Settings for "git commit" does not find the "Git: Input Validation" setting. Nor does searching for "commit length". That's why I couldn't find the right setting, to turn it back on.

[chrisant996]

And yes, the fact that "always" mode covers the Commit button is pretty problematic!

[hehaoqian]

Encountered the same issue.
Set

"git.inputValidation": "always"

or

"git.inputValidation": "warn"

solves my issue.

[chrisant996]

"git.inputValidation": "warn"

solves my issue.

Yes. But when always is chosen, the UI metaphor is broken, as has been mentioned in other issues.

Instead of the validation message in a floating box that covers other buttons (unfriendly to mouse users), the Commit Message box itself should expand to make room for the validation message box to be contained entirely within the Commit Message box.

[lszomoru]

@chrisant996, thank you very much for filing this issue. We did indeed change the default setting for validation in the latest release. I agree that I should have done a better job and documenting the change. I will make sure to update the release notes retroactively so that others can find the change more easily.

As far as the current validation UI, I do agree with all of you that it is less than ideal that it overlaps the primary action button. Since the commit message input is a full-fledged Monaco editor, I have been exploring using diagnostics information in order to signal a validation error (see the existing UI vs. the one based on diagnostics):

[chrisant996]

As far as the current validation UI, I do agree with all of you that it is less than ideal that it overlaps the primary action button. Since the commit message input is a full-fledged Monaco editor, I have been exploring using diagnostics information in order to signal a validation error (see the existing UI vs. the one based on diagnostics):

Ooo, that would be very nice indeed. No more fiddling to figure out where to manually apply wordwrap; the red squiggles would show exactly where at a glance.

[lszomoru]

Not only that, but this would also enable us to create a "code action" to wrap the text.

[cpsauer]

Similarly, it would be very nice if "warn" (& "always"?) would continue to show the first error if there is one, even if we leave the current interface.

I'd always assumed overlapping the button with an easily closable (tap anywhere) message was intentionally discouraging too-long commit messages when a length limit had been set.

Additional, perhaps setting a limit should automatically turn the validation on, if there's a good way to do that?

[ricardograca-scratch]

Not only that, but this would also enable us to create a "code action" to wrap the text.

Does that mean the message would be automatically formatted according to the line length settings? 🥹

[cgbur]

This is a weird default to want to change to off. Wouldn't it be better to have the default to 'warn' like previous to nudge people to follow the standard? I know when I first started I did not know git commit messages usually have a 50 character first line. Without this I would be blasting huge commit messages everywhere.

[Louis-7]

The default setting was "warn", right? It's beneficial to remind developers to summarise their commit title to less than 50 (or another reasonable length) so the title can be fully displayed everywhere without ellipsis. It also helps display a clean and neat commit log when you do git log --oneline. The length validation and warning message don't block us from doing more explanation in the commit body.

VS code is widely used, I think this default setting is instructive for most developers. If VS code doesn't do this validation by default there will be more and more people writing long commit messages which are difficult to read.

@lszomoru why did the team make this decision to change the default setting of this?

[rikkiprince]

@lszomoru If it's a full-fledged Monaco editor, would it be possible to render ruler lines at the git input validation widths?

[lszomoru]

@rikkiprince, you can do that using language specific editor settings. See #93150 (comment)

[chrisant996]

@lszomoru If it's a full-fledged Monaco editor, would it be possible to render ruler lines at the git input validation widths?

Won't word wrapping interfere? If the commit description editor width is narrower than the validation width(s), then word wrapping occurs. And so the input validation width can be at a different pixel position on each individual line. Even if Monaco editors put the ruler lines at the right pixel position on a per-line basis, there will be rule lines all over the place and they won't align.

[GrantGryczan]

I strongly agree that the default setting should be restored.

[chrisant996]

This is a matter of being a good git citizen.

It doesn't make sense why vscode has changed to use different defaults than git itself does.

If the issue was that the warning UI wasn't up to standards, then a preferable fix would be to improve the UI (e.g. any of the ways that have been discussed in this issue). Turning off the validation goes too far and is counter productive in the bigger picture of being a good git citizen.

[frozenzia]

.....I agree that I should have done a better job and documenting the change. I will make sure to update the release notes retroactively so that others can find the change more easily.

@lszomoru, fwiw, methinks this "updating the release notes" didn't get done - at least I found no mention of it. (Didn't help things that I was vacationing for a long time and didn't come across this til 1.86, but even so, couldn't find anything about it in the 1.85 relNotes.)

[verebes1]

@lszomoru For the past couple of months, I have been tearing my hair out why does the commit validation not work anymore? I have finally found a StackOverflow post relating to this which explained that this has been turned off. I am very happy to have it back on.
FWIW Entire teams are using this feature to write better commit messages that help others understand the changes faster without having to look at the diff. This setting has been enabled by default for years, so why change it?
I will be very grateful if the default warn will be brought back.
VSCode is such a fantastic IDE and this feature is absolutely amazing. I had feedback from other developers who have been literally taught by VSCode to write better commit messages thanks to this feature.
Once again thank you for the development of VSCode.

[lszomoru]

Today's VS Code Insiders release (version details below) contains a new setting (git.experimental.inputValidation) to enable the new experimental input validation mechanism for the commit input field that uses language diagnostics. This also enables us to enable quick fixes (ex: clear empty characters, hard wrap lines). Please enable the setting, give it a try and let me know your feedback.

Version: 1.87.0-insider (Universal)
Commit: 01f46bb5357baa8f3b9da690e1e34eb78e09a72b
Date: 2024-02-12T05:48:37.277Z
Electron: 27.3.1
ElectronBuildId: 26731440
Chromium: 118.0.5993.159
Node.js: 18.17.1
V8: 11.8.172.18-electron.0
OS: Darwin arm64 23.3.0

[lszomoru]

The git.experimental.inputValidation setting has been removed and the new diagnostics-based input validation can be enabled using the git.inputValidation. Give the new input validation a try and let me know your feedback.

[GrantGryczan]

I'm confused. This issue reported that a setting's default changed unexpectedly. How come you're asking for feedback on a new implementation of the setting under this issue's comments? Maybe I'm misunderstanding something. Does the new implementation address the concern this issue was trying to bring up? Has the default value been reverted to its expected value (warn)?

Edit: Got it! Didn't realize the new feature was a blocker for this issue.

[chrisant996]

@lszomoru the new input validation feedback is nice! But 2 questions:

1. Can you confirm that the default is/will be reverted back to warn?
2. Can it highlight only the part of the text that's a problem, instead of highlighting the entire line?

For example, here is what it just showed me a moment ago:

From what had been discussed here, I was expected to see the squiggle underline only apply to the characters that exceed the validation limit. Highlighting the entire line means I still manually do a binary search to find where I want to wrap it. Yes, I realize I could press Ctrl-. plus Enter to have it auto-wrap. But that's not ideal because sometimes I want to wrap differently to achieve a more pleasing view. Highlighting the actual overage would be very welcome.

UPDATE: I spoke too soon -- the new approach looks nice, but in practice it's actually worse than before, for me: previously it showed the count of characters over limit. Now it only shows the count if I move the mouse and hover over the squiggle and wait a second. So it takes more time and effort now, to apply wrapping. It's only a few seconds per line, but it adds up and it feels like swimming upstream.

Thanks for listening and for investing in improvements here!

[chrisant996]

@lszomoru Ok after using it for a couple of days, I still don't fully love it, but I've been able to change how I do things to compensate. Having changed how I form and capture my thoughts which typing specifically in that box, I'm able to reduce the effect so it's not as disruptive as it was at first.

I still think it would be much better to squiggle-underline only the characters that are over, though. I've adapted, but it's still slower and takes more cognitive concentration, which distracts from coming with content.

I.e. I used to focus on content, and then reformat after. But if I focus primarily on formatting and catch overages within a few letters, then I can minimize the time cost. But it distracts me from the content itself, and that's not great.

Just trying to share the thought process I experience, for context, in case that's helpful for choosing further refinements.

[lszomoru]

Thank you for the feedback. We will continue to listen to additional feedback and adjust the validation UX based on feedback. @GrantGryczan, yes we have discussed reverting the default value of the setting but there are still couple of issues that need to be sorted out.

[yCodeTech]

I actually preferred the previous dismissable message box as displayed in the image below, thanks to this SO post for the image.

It was always on display as a reminder. As @chrisant996 pointed out the new squiggle underline takes time to hover over it, to even see what's wrong with it. The squiggle doesn't immediately convey the error. It could be any kind of error, like a spelling error etc.

What's very strange to me is that this change has only occurred recently, at least for me. I'm pretty sure I saw the old message box just last week. And what's more, it is very unprofessional to change the UI very suddenly without warning, or consulting it's users via the software's interface (not everyone keeps updated on these issues!).

There was absolutely nothing wrong with the message box, and I highly recommend you reinstate it. If you want, make it an option to change the styling of error, letting us users decide whether to use the message box or the awful (in my opinion) squiggle underline.

[verebes1]

I actually preferred the previous dismissable message box as displayed in the image below, thanks to this SO post for the image.

It was always on display as a reminder. As @chrisant996 pointed out the new squiggle underline takes time to hover over it, to even see what's wrong with it. The squiggle doesn't immediately convey the error. It could be any kind of error, like a spelling error etc.

What's very strange to me is that this change has only occurred recently, at least for me. I'm pretty sure I saw the old message box just last week. And what's more, it is very unprofessional to change the UI very suddenly without warning, or consulting it's users via the software's interface (not everyone keeps updated on these issues!).

There was absolutely nothing wrong with the message box, and I highly recommend you reinstate it. If you want, make it an option to change the styling of error, letting us users decide whether to use the message box or the awful (in my opinion) squiggle underline.

Same here I really preferred the dismissable message box. Please could you bring it back. Why change something that was working and everyone loved it?

[chrisant996]

What were the problems with the warning message box that led to removing it?

I think the following would be nice:

1. Optionally (and enabled by default) show a warning message box as before about the current line containing the caret, AND
2. Optionally (and enabled by default) show a squiggle only under the characters that exceed the limit on each line visible in the edit box, AND
3. Optionally (and enabled by default) highlight the letters past the limit in red (make the color configurable if necessary).

• The three settings above could be separate Boolean options. (Alternatively they could be a single combined enum option, but that gets messy quickly.)
• Plus there could be a tiny icon button next to the Commit button, to control the commit length validation feature, e.g. to drop down a menu to the the options. Discoverability is poor otherwise, especially if the feature is going to be disabled by default in option to git itself and how other editors behave.
• Plus the feature name or description in Settings could be something like "git commit message validation" to make it actually findable when searching the settings ("input" or "validation" aren't intuitive search terms for someone who doesn't know the name of the feature in advance -- "commit" and "message" are natural search terms).

[Louis-7]

I prefer the display a warning box under the commit input box either. I'm using Code Spell Checker it can help me correct the typos in the commit message.

As you can see, it marks the typo with a wavy line. But the new style overrides the Code Spell Checker and I have to move my mouse over the line to read the warning message.

Displaying a warning box in the area might not be the best solution however it works for a long time and is pretty handy.

[GrantGryczan]

I think it would be great if it only underlined the portion of the text that exceeds the line's limit. Then you wouldn't need to hover to see the length! And it would be more convenient than the old design, since it would actually show where you need to wrap or shorten text, as opposed to needing to figure it out by trial and error, counting characters, or guessing.

The previous comment would also be less of a problem, since an underline would only appear over the last part of the text.

[thengstermanndev]

Please bring the message box back 🙏