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

Revert selected lines context option #3159

Closed
TamaMcGlinn opened this issue Apr 4, 2016 · 16 comments
Closed

Revert selected lines context option #3159

TamaMcGlinn opened this issue Apr 4, 2016 · 16 comments
Assignees
Labels
type: feature request up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/
Milestone

Comments

@TamaMcGlinn
Copy link

When viewing a commit, I'd like to be able to select lines from that commit and have an option in the right-click context menu for "Revert selected lines".

What I currently do for this is to revert the commit, then stage those lines and remove the rest.

Just like cherry pick, the default should be to make staged changes out of the lines, with a checkbox for "Automatically commit".

@KindDragon KindDragon added type: feature request up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/ labels Apr 27, 2016
@jbialobr jbialobr self-assigned this Apr 21, 2017
@jbialobr jbialobr added this to the 2.50 milestone Apr 24, 2017
@PierrotG
Copy link

PierrotG commented Jul 5, 2017

Hi, there is one place where the "revert selected" is missing: in the diff window, file context menu, the user can right click on a file and select "Cherry-pick file's changes" but there is no "Revert selected file's changes" option.

The only way to perform this action is to reverse the order of the diff (select the child first then the parent) and then cherry-pick the file changes (so, one is actually cherry-picking the reverse diff changes), which is not very intuitive.

@jbialobr
Copy link
Member

jbialobr commented Jul 7, 2017

@PierrotG It was added in the 2.50 version.

@PierrotG
Copy link

PierrotG commented Jul 7, 2017

@jbialobr
Hi, I'm indeed talking about the 2.50.00 version.
In the Diff window, right click on a file, you get the following options:

  • Open with difftool >
  • Save (B) as... Ctrl+S
  • Reset file(s) to >
  • Cherry pick file's changes
  • Copy full path(s) Ctrl+C
  • Open containing folder(s)
  • Show in File tree
  • File History
  • Blame
  • Find Ctrl+F

There is no "Revert file's changes" listed, and, as I commented before, it would be nice to have it, the same way we get "Revert Commit" for commits and "Revert selected lines" for individual chunks.
As I commented above, it is still possible to emulate this behaviour by inverting the diff and then use "Cherry pick file's changes", but this is not obvious.

@jbialobr
Copy link
Member

jbialobr commented Jul 7, 2017

Ok, you are right, I misread your comment, having the issue title in mind. Since 2.50 you can select a file and then select all changes in the file's diff view and choose from the context menu to revert the selected lines.
Please raise a separate issue that we could close it when it gets implemented.

@PierrotG
Copy link

PierrotG commented Jul 7, 2017

@jbialobr Sure no problem. I created the issue, #3824. Sorry about the confusion.

@RussKie
Copy link
Member

RussKie commented Nov 12, 2018

"Reset file(s)..." allows you to revert to the previous commit.

@RussKie RussKie reopened this Nov 12, 2018
@RussKie RussKie closed this as completed Nov 12, 2018
@Frettman
Copy link

Is there a regression or am I looking at the wrong place? I select a commit, select a file in the "Diff" tab and right click on any of the changed lines (with and without selecting a range of text), but there is no "Revert" option. The first two available options are "Stage selected line(s)" and "Reset selected line(s)" which don't seem to make much sense here. And they either cause a "patch failed" error or make some local changes before immediately triggering a merge conflict.

@gerhardol
Copy link
Member

This was changed in 3.0 or so.
"Reset" is more in line with what Git uses; it is common to use reset to change back to a specific version and revert/cherry-pick for commits.
Stage could have been apply instead, but changes are always added to the index and for worktree/index commits unstage is used.

@Frettman
Copy link

I see, thank you. So "Reset selected line(s)" should be what I'm looking for. But there's still the issue of the constant "error: patch failed" when trying to reset lines from commits. Although, at least when there are no changes yet in that file, it actually stages the reverted lines despite the error. But reverting more changes from that file mostly doesn't fails with the same error. But that's probably a different issue. At least now I know how it should work.

@pmiossec
Copy link
Member

But reverting more changes from that file mostly doesn't fails with the same error. But that's probably a different issue

@Frettman this feature doesn't play well when the diff is displayed with some 'ignore whitespace changes'. When you want to use it, most of the time, disabling it before resetting the changes is the way to go...

I think that's the problem you have...

@Frettman
Copy link

Unfortunately I have tried that already; even "Show entire file". I have tried resetting one-liner changes from the latest commit (to definitely avoid conflicts with any intermediate commits), but it's still the same. I have tried a fresh install on a new machine. I have also verified that the file encoding is correct. I have checked line endings (it's LF with Git configured to core.autocrlf to "input"). No idea what else to check.
Whatever I try to reset, I always get the "error: patch failed" followed by "Falling back to direct application...". That fallback actually seems to work as long as the changes aren't too close to other already staged changes. So basically it looks like I can only reset once per block shown in the diff (without "Show entire file" on). So I have to think ahead a bit what changes of a block I want to reset; I can do that ;) But I still get the error every time, even if the fallback works, which is a bit annoying.

@RussKie
Copy link
Member

RussKie commented Aug 21, 2022

Try it in reverse - unstage the file completely, and then only stage what's relevant.

@Frettman
Copy link

The context is reverting individual lines from a commit, where there is no unstage? Unless you mean resetting the whole file and then resetting all the staged changes I don't want to revert. That's the workaround the OP used back in 2016 and yes, it's still valid :)

@mstv
Copy link
Member

mstv commented Aug 25, 2022

A screenshot might help to talk about the same thing.
Usually whitespace is the "blocker". ("Entire file" is unrelated.)
If you want to revert a single line from a changed block, git might be too defensive. Then an external diff tool might help:

image

@Frettman
Copy link

Here are two screenshots illustrating the most basic use case I could find:
I select the latest commit that only contains a single changed line. There's no whitespace changes. I select that change in the Diff tab and pick "Reset selected line(s)" from the context menu.
image
This results in the following error message:
image
The "Falling back to direct application..." seems to work, so ultimately the line was reverted despite showing an error. And this error message basically looks the same like the error when even the fallback fails and nothing is reset. So maybe just don't show an error if the fallback works?

Anyway, I will try using an external diff tool in the future. Thanks for that tip.

@gerhardol
Copy link
Member

The "Falling back to direct application..." seems to work, so ultimately the line was reverted despite showing an error. And this error message basically looks the same like the error when even the fallback fails and nothing is reset. So maybe just don't show an error if the fallback works?

If the Git retry works, the popup is removed in 4.0.

The reset/stage handling works by creating the equivalence of patchfiles, so if a patchfile would file, the line patching will fail.
As mentioned above, hiding whitespace changes is such a case that could cause the patch to fail. GE cannot guess if you really want whitespace changes (that would also require that another git-diff got the unmodified lines before ignoring whitespaces).

The algoritm when applying patches could still be tuned and let GE retry some more. The patching requests that Git "tries its hardest" but then Git requires that a file is identical in worktree and index "commits". So there could be separate attempt if the first try failed.

Regardless what is can be done with these patches: Sometimes you know better how to apply the changes, use an external difftool then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/
Projects
None yet
Development

No branches or pull requests

9 participants