Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Discard lines in file diff #487

Merged
merged 60 commits into from
Feb 10, 2017
Merged

Discard lines in file diff #487

merged 60 commits into from
Feb 10, 2017

Conversation

kuychaco
Copy link
Contributor

@kuychaco kuychaco commented Jan 26, 2017

Fixes #50.
Fixes #481.

This PR implements discarding hunk lines via a Discard Selection context menu option and allows users to undo discard actions. This option is in a separate context menu section to reduce the possibility of accidentally selecting it, per #481 (comment).

You can undo discards via the Undo Last Discard button (see below), keybinding cmd-z, or command github:undo-last-file-diff-discard.

discarding-lines

The undo action will fail if the text buffer is modified, or if the file has been changed since the discard action.

undoing-failure

Additionally, this PR introduces a new file diff header bar that displays the corresponding file name and staging status, and has buttons for opening the file in an editor and viewing the corresponding staged/unstaged version of the file if one exists.

new-header

TODO:

  • fix failing tests. one consistently failing and on flakey
  • highlight corresponding file when jumping between staged/unstaged versions of same file (or would this be too jarring?)
  • add Stage / Unstage all button in file diff header?

removedCount++;
} else {
// eslint-disable-next-line no-console
console.error(buffer.lineForRow(row) + ' does not match ' + line.text);
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥 ?

@simurai
Copy link
Contributor

simurai commented Jan 27, 2017

ok, yess... Be able to switch to the other list feels nice.. 👌

toggle

@ungb
Copy link

ungb commented Feb 8, 2017

from some adhoc testing, I found a edge case.

  1. Make some changes to a file and save.
  2. go to unstage changes and open the Unstage Changes view of the file
  3. Discard all the changes you made by selecting all the hunks and clicking discard selection.
  4. File disappears from Unstaged Changes (expected)
  5. Make a change to the same file and save
  6. Open the file.

Expected: Shouldn't have the ability to Undo Last Discards since all changes were already discarded and it's a new edit.

Actual: Unstage Changes has Undo Last Discards, but when you click on it, it says the file has been edited.

@kuychaco
Copy link
Contributor Author

kuychaco commented Feb 9, 2017

@ungb thanks for testing and bringing up this edge case! This one's tricky because in theory we can actually allow people to undo their last discard even if the file has been changed since the discard.

One thing to keep in mind is that the text buffer's undo history and the undo last discard history for the file diff are completely separate mechanisms and thus one can theoretically make changes to a buffer and still be able to undo discards.

We can in theory undo a discard even after the file has been modified by applying a patch that has the discarded lines to working directory version of the file. If the discarded lines are in a different part of the file than the new modifications then the patch should apply cleanly without conflicts.

I say "in theory" because we haven't yet implemented a version of this feature that allows users to undo their last discard if the file has been modified since the discard. This was for the sake of shipping something quick and simple up front, but there's certainly room for us to iterate and make the undo last discard feature even more flexible.

@kuychaco kuychaco merged commit b11e36b into master Feb 10, 2017
@kuychaco kuychaco deleted the ku-discard-lines branch February 10, 2017 03:06
simurai added a commit that referenced this pull request Feb 10, 2017
kuychaco added a commit that referenced this pull request Feb 11, 2017
Due to bug in discard logic #509

This reverts commit b11e36b, reversing
changes made to 8e09a04.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discard changes should warn user of action Add ability to discard
5 participants