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

If No Staged Files, Changed Files are Added to Commit #15613

Closed
atolken opened this Issue Nov 17, 2016 · 14 comments

Comments

Projects
None yet
9 participants
@atolken

atolken commented Nov 17, 2016

  • VSCode Version: 1.71
  • OS Version: Windows 10 Enterprise

Steps to Reproduce:

  1. make changes / have changed files
  2. do not stage any files
  3. write commit message
  4. submit commit

problem: all files that were changed (even though none were staged) are added to the commit.
expected: if no files are staged, the commit will not add any files (similar to other git tools).

@siegebell

This comment has been minimized.

Show comment
Hide comment
@siegebell

siegebell Nov 17, 2016

This behavior is likely intentional, but after many revert-last-commit operations, I've come to really hate it.

siegebell commented Nov 17, 2016

This behavior is likely intentional, but after many revert-last-commit operations, I've come to really hate it.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Nov 17, 2016

Member

This is intended.

  • when there are no changes, nothing happens
  • when there are no staged changes, all working tree changes are committed
  • when there are staged changes, only those are committed

Would you like a setting to disable that?

Member

joaomoreno commented Nov 17, 2016

This is intended.

  • when there are no changes, nothing happens
  • when there are no staged changes, all working tree changes are committed
  • when there are staged changes, only those are committed

Would you like a setting to disable that?

@joaomoreno joaomoreno added this to the Backlog milestone Nov 17, 2016

@joaomoreno joaomoreno added the bug label Nov 17, 2016

@atolken

This comment has been minimized.

Show comment
Hide comment
@atolken

atolken Nov 17, 2016

I would definitely like a setting to disable that, please.

atolken commented Nov 17, 2016

I would definitely like a setting to disable that, please.

@MaxfieldWalker

This comment has been minimized.

Show comment
Hide comment
@MaxfieldWalker

MaxfieldWalker Nov 25, 2016

In the case of Visual Studio, when there are no staged changes, but some changes, the content of the commit button comes "Commit All". So, user can know all changes will be committed even no changes staged.

vscode doesn't have button to commit (use ctrl+Enter instead), and it is difficult to know what happens when commit done.

I would like to add a setting to disable the behavior when no changes staged, or add some ingenuity to let user know what will happen.

image

image

MaxfieldWalker commented Nov 25, 2016

In the case of Visual Studio, when there are no staged changes, but some changes, the content of the commit button comes "Commit All". So, user can know all changes will be committed even no changes staged.

vscode doesn't have button to commit (use ctrl+Enter instead), and it is difficult to know what happens when commit done.

I would like to add a setting to disable the behavior when no changes staged, or add some ingenuity to let user know what will happen.

image

image

@seesemichaelj

This comment has been minimized.

Show comment
Hide comment
@seesemichaelj

seesemichaelj Mar 31, 2017

Contributor

Despite the fact that this is easily revertible after an accidental mass commit, the git CLI does not allow you to commit without staged files.

If it's not a setting that disables this functionality, then if there are no staged files, prompt the user that you cannot commit without staged changes and ask if the user would like to stage and commit all changes (with the default option to be No).

Forcing the user to look at the files and intentionally stage them is good practice.

Contributor

seesemichaelj commented Mar 31, 2017

Despite the fact that this is easily revertible after an accidental mass commit, the git CLI does not allow you to commit without staged files.

If it's not a setting that disables this functionality, then if there are no staged files, prompt the user that you cannot commit without staged changes and ask if the user would like to stage and commit all changes (with the default option to be No).

Forcing the user to look at the files and intentionally stage them is good practice.

@mozram

This comment has been minimized.

Show comment
Hide comment
@mozram

mozram Apr 20, 2017

Any progress on this issue? As @siegebell said before, I had to revert commit many times already as I either accidentally pressed CTRL+S or forgot to stage file. I'm hating it now.

@seesemichaelj suggestion is very good actually.

mozram commented Apr 20, 2017

Any progress on this issue? As @siegebell said before, I had to revert commit many times already as I either accidentally pressed CTRL+S or forgot to stage file. I'm hating it now.

@seesemichaelj suggestion is very good actually.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Apr 25, 2017

Member

If it's not a setting that disables this functionality, then if there are no staged files, prompt the user that you cannot commit without staged changes and ask if the user would like to stage and commit all changes (with the default option to be No).

I like the way you're thinking. Let's create a git.enableSmartCommit setting, defaults to false.

We can then present the following message, in these cases: There are no staged changes to commit. Would you like to stage all changes and commit them?. The options are:

  • Yes
  • Always
  • Cancel

Always would change that setting to true.

Anyone cares to submit a PR?

Member

joaomoreno commented Apr 25, 2017

If it's not a setting that disables this functionality, then if there are no staged files, prompt the user that you cannot commit without staged changes and ask if the user would like to stage and commit all changes (with the default option to be No).

I like the way you're thinking. Let's create a git.enableSmartCommit setting, defaults to false.

We can then present the following message, in these cases: There are no staged changes to commit. Would you like to stage all changes and commit them?. The options are:

  • Yes
  • Always
  • Cancel

Always would change that setting to true.

Anyone cares to submit a PR?

@seesemichaelj

This comment has been minimized.

Show comment
Hide comment
@seesemichaelj

seesemichaelj Apr 26, 2017

Contributor

I'll give this a shot

Edit: I believe the below pull request (#25364) is how this is supposed to be implemented.

Contributor

seesemichaelj commented Apr 26, 2017

I'll give this a shot

Edit: I believe the below pull request (#25364) is how this is supposed to be implemented.

@ashirley

This comment has been minimized.

Show comment
Hide comment
@ashirley

ashirley May 3, 2017

I have been looking at this and related issues (before I saw you were too @seesemichaelj , sorry!) and have attempted a solution from a slightly different direction and I have raised PR #25855. I think there is value in trying to merge our two solutions which I will have a look at tomorrow if I can.

ashirley commented May 3, 2017

I have been looking at this and related issues (before I saw you were too @seesemichaelj , sorry!) and have attempted a solution from a slightly different direction and I have raised PR #25855. I think there is value in trying to merge our two solutions which I will have a look at tomorrow if I can.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno May 4, 2017

Member

Sorry @ashirley, I've closed your PR. I really liked @seesemichaelj's fix, it's simpler to understand and maintain.

Member

joaomoreno commented May 4, 2017

Sorry @ashirley, I've closed your PR. I really liked @seesemichaelj's fix, it's simpler to understand and maintain.

@joaomoreno joaomoreno modified the milestones: May 2017, Backlog May 4, 2017

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno May 4, 2017

Member

Fixed by #25364

Member

joaomoreno commented May 4, 2017

Fixed by #25364

@joaomoreno joaomoreno closed this May 4, 2017

@ashirley

This comment has been minimized.

Show comment
Hide comment
@ashirley

ashirley May 5, 2017

No worries. I am a bit concerned that when you press "Always", it changes a file in the directory and then immediately commits it. I wouldn't be expecting this as a user and would want a chance to add the (potentially new) file to .gitignore. Even if I did want to commit this config change, I would want it in a separate commit.

ashirley commented May 5, 2017

No worries. I am a bit concerned that when you press "Always", it changes a file in the directory and then immediately commits it. I wouldn't be expecting this as a user and would want a chance to add the (potentially new) file to .gitignore. Even if I did want to commit this config change, I would want it in a separate commit.

@seesemichaelj

This comment has been minimized.

Show comment
Hide comment
@seesemichaelj

seesemichaelj May 7, 2017

Contributor

@ashirley I agree and verified what you said. I have opened Issue #26169 to address this

EDIT: #26169 was closed because it is no longer an issue (I verified your issue using an outdated rebase of master). @joaomoreno changed the function to use global settings instead of workspace settings in commit b31c1e1. Therefore, the project's settings.json file will not be created/edited upon clicking Always.

Contributor

seesemichaelj commented May 7, 2017

@ashirley I agree and verified what you said. I have opened Issue #26169 to address this

EDIT: #26169 was closed because it is no longer an issue (I verified your issue using an outdated rebase of master). @joaomoreno changed the function to use global settings instead of workspace settings in commit b31c1e1. Therefore, the project's settings.json file will not be created/edited upon clicking Always.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno May 8, 2017

Member

Good detective work @seesemichaelj 😉

Member

joaomoreno commented May 8, 2017

Good detective work @seesemichaelj 😉

@sandy081 sandy081 added the verified label Jun 1, 2017

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.