Fix #63127 - Disable smart commit message box

[skprabhanjan]

@joaomoreno , Added "enableSmartCommitWarning" Setting to fix #63127 .
Please review this and let me know if there are any changes :)

[V-ed]

I'm not a member of vscode so I'm just giving my two cents here :)

Is not showing the enable smart commit warning a regression? New users might not know smart commit exists if a prompt is not shown.

Maybe there could be a third option for the warning message which updates the new setting you added : an option "Never".

That option, as already mentionned, would give the possibility for the user to not see that warning again yet still allow for controls by settings if desired.

Also, it would feel weird to go activate a setting (git.enableSmartCommitWarning) that is a warning that would then never be shown again because that warning simply activates the git.enableSmartCommit setting (at this point simply turning on smart commit would be faster, in other words).

Edit : Someone else took the work (#63743) despite you saying you wanted to work on it, and his implementation is basically my review in a nutshell. I prefer your setting name personally, but @joaomoreno will decide on this, I have no authority to decide here :)

[V-ed]

This would therefore be true

[skprabhanjan]

@V-ed , Thanks for the review :)
And I never saw that someone else sent a PR for this until I sent one .
But yeah I get your point, if @joaomoreno , suggests to add a third option called 'never' then i shall go ahead and make the changes.

Also, it would feel weird to go activate a setting (git.enableSmartCommitWarning) that is a warning that would then never be shown again because that warning simply activates the git.enableSmartCommit setting (at this point simply turning on smart commit would be faster, in other words).

This has been my confusion from the start, why do we need two separate settings(As stated "turning on smart commit would be faster" ) and git.enableSmartCommitWarning would make much less sense if we add a "Never" Option as suggested.
These are my thoughts.
Thanks :)

[mIkhail-zaretsky]

Hi,
It was me who sent a PR, I'm sorry for this - it was not polite from my side. I just wanted to introduce an idea of how it can be implemented.

[V-ed]

@skprabhanjan

why do we need two separate settings(As stated "turning on smart commit would be faster" ) and git.enableSmartCommitWarning would make much less sense if we add a "Never" Option as suggested.

Actually, there would be a valid reason that comes to my mind on why it would still be useful to have as a setting : syncing settings to multiple / new machines.

I know that such warning could be remembered internally, but will that internal state be remembered when switching machines? I'm guessing not - therefore, settings can be easily shared and used as the state of the warning.

I think it's already how VSCode's other types of "Do not show again" notification messages stores their state (in the settings), but I'm not entirely sure, so don't quote me on that.

[skprabhanjan]

Hi,
It was me who sent a PR, I'm sorry for this - it was not polite from my side. I just wanted to introduce an idea of how it can be implemented.

@mIkhail-zaretsky , That's fine :)

I know that such warning could be remembered internally, but will that internal state be remembered when switching machines? I'm guessing not - therefore, settings can be easily shared and used as the state of the warning.

@V-ed I understand this point but when you add an option like "Never" and click on it (As discussed before) and later enable this enableSmartCommitWarning setting, isn't this something confusing (What should ideally happen ?)

[V-ed]

@skprabhanjan I think the name of the command is already self-explanatory (which is also why I said in my first comment that I preferred your setting name than @mIkhail-zaretsky's implementation) : It enables the warning again, warning that is shown only when smart commit is not enabled. I see it simple as that. It's kind of like "I pressed Never but changed my mind, maybe someday I'd like to enable smart commit, keep warning me"

Yes, it would mean this setting wouldn't do anything if smart commit is enabled - I don't think this is an issue however.

Also, just to make sure we're on the same page, pressing "Never" would mean that your new setting would be set to false without the user having to go change it himself.

[skprabhanjan]

Also, just to make sure we're on the same page, pressing "Never" would mean that your new setting would be set to false without the user having to go change it himself.

@V-ed , Okay I am clear now :)
Will change it accordingly , thanks :)

[skprabhanjan]

@joaomoreno , Any updates on this ?

[joaomoreno]

@skprabhanjan Sorry for the delay. I'm currently sitting on a big backlog, so things are taking its time, but I'll get to it. Feel free to reopen.

[skprabhanjan]

@joaomoreno , Sorry for that and I understand it now , Thanks for getting back.
Will reopen it :)

[skprabhanjan]

@joaomoreno , any updates ?
This is pending from a long time , please review this :)

[joaomoreno]

Went with #63743 Thanks anyway! 🙏