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

stage files from merge group properly #5456

Merged
merged 1 commit into from
Jun 19, 2019
Merged

stage files from merge group properly #5456

merged 1 commit into from
Jun 19, 2019

Conversation

akosyakov
Copy link
Member

fix #5455

How to test

@akosyakov
Copy link
Member Author

cc @32leaves

@kittaakos
Copy link
Contributor

I could not spot any difference between the changes from here and the master; I ran into an error with both states: #5463. Other than the error, when I staged and unstaged the files, both behaved the same.

@akosyakov
Copy link
Member Author

akosyakov commented Jun 14, 2019

You should be able to stage from merge conflicts. On master it does not work for individual files. It can only stage from changes.

const { repository, unstagedChanges } = this;
if (unstagedChanges.some(change => change.uri === uri)) {
const { repository, stagedChanges } = this;
if (!stagedChanges.some(change => change.uri === uri)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the way it was before should be good. this line breaks it. can you explain the intent, please?

2019-06-14 07 31 35

I cannot stage this new change to a file which already was staged before.

Copy link
Contributor

@AlexTugarev AlexTugarev Jun 14, 2019

Choose a reason for hiding this comment

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

Maybe we should express the condition clearly; is this what you wanted to check for?

const dirty = unstagedChanges.some(change => chan`ge.uri === uri) || mergeChanges.some(change => change.uri === uri);

Copy link
Contributor

Choose a reason for hiding this comment

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

just tested quickly; with the proposed change I could stage changes for already staged files and it played well with the merge conflict changes.

@akosyakov
Copy link
Member Author

akosyakov commented Jun 14, 2019

@AlexTugarev @kittaakos please feel free to open an alternative PR, i won't be around today

Co-authored-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Also-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Alex Tugarev <alex.tugarev@typefox.io>
@AlexTugarev
Copy link
Contributor

@kittaakos, I've update this PR. Please have a look.

@kittaakos
Copy link
Contributor

I cannot stage this new change to a file which already was staged before.

It worked. 👍

Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

I am approving this; however, I was not able to reproduce the original issue from the master.

@akosyakov
Copy link
Member Author

just had it again, pretty annoying that it is not possible to stage one file after another, merging it

@akosyakov akosyakov merged commit 531aa3b into master Jun 19, 2019
@akosyakov akosyakov deleted the GH-5455 branch June 19, 2019 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot stage merged changes
3 participants