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

Add index to unstage command #11635

Merged

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Sep 2, 2022

What it does

Fixes #11633

How to test

  1. Open a Git repository.
  2. Modify several files.
  3. Stage one or more of them.
  4. Unstage one of the staged files using the SCM UI.
  5. That single file should be unstaged, other staged files should remain staged, and the content of no file should be changed.

  1. Try commands in the SCM widget at different levels (Resource Group, Folder)
  2. Confirm that they behave as expected.

Review checklist

Reminder for reviewers

Co-authored-by: Colin Grant <[email protected]>
Co-authored-by: Vince Fugnitto <[email protected]>
@colin-grant-work colin-grant-work added the git issues related to git label Sep 2, 2022
@jaredkipe
Copy link

I could be wrong, but there are multiple this.git.unstage(repository, uris) calls in the file that seem like they may benefit from this solution. I tried patching the file in Docker but I was unsuccessful without just reverting the library itself. So I cannot 'review' it as working.

@vince-fugnitto
Copy link
Member

I could be wrong, but there are multiple this.git.unstage(repository, uris) calls in the file that seem like they may benefit from this solution.

@jaredkipe there is only one other git.unstage call I can identify and it correctly uses the default all option to unstage all:

async unstageAll(): Promise<void> {
try {
const { repository, stagedChanges } = this;
const uris = stagedChanges.map(c => c.uri);
await this.git.unstage(repository, uris);
} catch (error) {
this.gitErrorHandler.handleError(error);
}
}

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm it works as expected 👍 I'll give @msujew a chance to review as well.

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍 I can confirm that the issue exists on master and is addressed by this change.

@colin-grant-work colin-grant-work merged commit 9ab4618 into eclipse-theia:master Sep 7, 2022
@colin-grant-work colin-grant-work deleted the bugfix/git-unstage branch September 7, 2022 15:48
@colin-grant-work colin-grant-work added this to the 1.30.0 milestone Sep 14, 2022
CareyJWilliams pushed a commit to ARMmbed/theia that referenced this pull request May 23, 2023
Co-authored-by: Colin Grant <[email protected]>
Co-authored-by: Vince Fugnitto <[email protected]>
CareyJWilliams pushed a commit to ARMmbed/theia that referenced this pull request May 23, 2023
Co-authored-by: Colin Grant <[email protected]>
Co-authored-by: Vince Fugnitto <[email protected]>
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Jun 5, 2023
Co-authored-by: Colin Grant <[email protected]>
Co-authored-by: Vince Fugnitto <[email protected]>
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Jun 12, 2023
Co-authored-by: Colin Grant <[email protected]>
Co-authored-by: Vince Fugnitto <[email protected]>
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Jun 12, 2023
Co-authored-by: Colin Grant <[email protected]>
Co-authored-by: Vince Fugnitto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
git issues related to git
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git/SCM - Unstage resets all files!
4 participants