Skip to content
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

Fix Microsoft #71258 - Cannot undo empty commits #72699

Merged
merged 3 commits into from Aug 8, 2019

Conversation

@marmikc
Copy link
Contributor

commented Apr 22, 2019

This is a fix for #71258

I've noticed that in this bug there are essentially two parts to it. If VS Code tries to undo a commit where there was an empty commit message, or a commit message with only whitespace, the "Bad Commit" error occurs.

Furthermore, there is a way to make a commit with just whitespace through VS Code. If you press the tick by Source Control: Git, enter some whitespace, and then press enter, the commit goes through with the message being whitespace. If you try to undo this, the same error will occur.

  • Fix: Cannot undo git commits with only whitespace
  • Fix: Can make commits with only whitespace
    • I have put this as an error because when filling out a message for a commit in the side bar if there is only whitespace, it will refuse commits. Hence going by that, I am extrapolating that to mean that any commit messages drafted in the top bar should refuse whitespace too.

I've removed the second as a requirement because the issue does not explicitly call for a fix on that.

UndoGitCommits

By testing with different commits it seems the regex matching expression is at fault.

The string that goes into the regex match is as follows:

[hash of previous git commit]
emailaddress@domain.com
[hash of this git commit]
[name of this commit]

The last part, the name of this commit, is optional, and is not present when there is an empty git commit, but the regex expression looks for it, and hence sees the expression as invalid.

Finally the change in commands.ts makes it so that when whitespace is added as a commit message, the function returns and the window is dismissed. This is the same behaviour as when Enter is pressed (without typing anything) and the window is dismissed. Below is a screenshot of the window.

whitespace_commit_part2

When enter is pressed, the window is dismissed.

The above has been reverted to its original behaviour.

marmikc added 2 commits Apr 22, 2019
Disallow making commits that contain only contain whitespace to be co…
…nsistent across other methods of committing to git through VS Code
Remove part of the regex matching expression for git commits, because…
… previous it would think commits with empty messages were invalid when infact they are not

@marmikc marmikc changed the title Fix Microsoft#71258 - Cannot undo empty commits Fix Microsoft #71258 - Cannot undo empty commits Apr 22, 2019

@marmikc

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

If anyone could guide me on what's causing these failure in check I would appreciate it. I am unsure of what is going wrong and how to fix it.

@octref octref requested a review from joaomoreno Apr 22, 2019

Revert earlier commit that disables creating a commit with just white…
…space, since this is not part of the original issue
@marmikc

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

I reverted an earlier commit I made that prevented the user from being able to create commits with only whitespace. I figured since this was explicitly not part of the original issue, it is something that should be changed in a different place.

@joaomoreno joaomoreno added the git label May 3, 2019

@joaomoreno joaomoreno added this to the Backlog milestone May 3, 2019

@joaomoreno
Copy link
Member

left a comment

Good investigation but the fix breaks the Undo behavior of populating the input box with the commit message. By removing that 4th capture group, you lost the commit message for commits which do have a message. I've pushed daf581d instead.

I've merged your PR anyway, since you drove me to the right fix. Thanks! 🍻

@joaomoreno joaomoreno merged commit 6829b21 into microsoft:master Aug 8, 2019

5 checks passed

VS Code Build #20190422.24 succeeded
Details
VS Code (Linux) Linux succeeded
Details
VS Code (Windows) Windows succeeded
Details
VS Code (macOS) macOS succeeded
Details
license/cla All CLA requirements met.

@joaomoreno joaomoreno modified the milestones: Backlog, August 2019 Aug 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.