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.

Sign up for GitHub

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

Jump to bottom

implement IEditorInput#confirm #153211

Merged
merged 1 commit into from
Jun 26, 2022
Merged

implement IEditorInput#confirm #153211

merged 1 commit into from
Jun 26, 2022

Conversation

jrieken
Copy link
Member

@jrieken jrieken commented Jun 25, 2022 •
edited
Loading

implement IEditorInput#confirm to confirm with users that it is OK to close a merge editor despite unresolved conflicts. As a side-effect keep the merge editor dirty while it is unsed and while it has unresolved conflicts

fixes #151024

@jrieken
implement IEditorInput#confirm to confirm with users that it is OK …
cdfdb50 
…to close a merge editor despite unresolved conflicts. As a side-effect keep the merge editor dirty while it is unsed and while it has unresolved conflicts

fixes #151024
@jrieken jrieken self-assigned this Jun 25, 2022
@jrieken jrieken enabled auto-merge (squash) June 25, 2022 16:20
@vscodenpa vscodenpa added this to the June 2022 milestone Jun 25, 2022
@jrieken jrieken mentioned this pull request Jun 25, 2022
Closed
@jrieken jrieken changed the title implement IEditorInput#confirm to confirm with users that it is OK to close a merge editor despite unresolved conflicts. As a side-effect keep the merge editor dirty while it is unsed and while it has unresolved conflicts implement IEditorInput#confirm Jun 25, 2022
bpasero
bpasero approved these changes Jun 26, 2022
View reviewed changes
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

👍

@jrieken jrieken merged commit 60b7514 into main Jun 26, 2022
@jrieken jrieken deleted the joh/foolish-penguin branch June 26, 2022 05:06
@bpasero
Copy link
Member

bpasero commented Jun 26, 2022

@jrieken one follow up maybe would be to emit onDidChangeDirty from within the mergeEditorInput when the state of conflicts transitions from N to 0 or 0 to N (with N gt. 1). In other words, since dirty state in a merge editor now depends on both model being dirty and conflicts being there or not, it would be good to add this condition to the dirty change event as well. This event is used in the editor part to update the dirty indicator.

@jrieken
Copy link
Member Author

jrieken commented Jun 27, 2022

@jrieken one follow up maybe would be to emit onDidChangeDirty from within the mergeEditorInput when the state of conflicts transitions from N to 0 or 0 to N (with N gt. 1).

We actually do that here

@bpasero
Copy link
Member

bpasero commented Jun 27, 2022

Ah ok, I saw that autorun code thingy and was not entirely sure what that is for ;)

@github-actions github-actions bot locked and limited conversation to collaborators Aug 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bpasero bpasero bpasero approved these changes

Assignees

@jrieken jrieken

Labels
None yet
Projects
None yet
Milestone
June 2022
Development

Successfully merging this pull request may close these issues.

No prompt when accepting merges with conflicts
3 participants
@jrieken @bpasero @vscodenpa