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

[Search git status] preview changed files #237

Merged
merged 12 commits into from
Jun 13, 2022
Merged

Conversation

NextAlone
Copy link
Contributor

@NextAlone NextAlone commented May 10, 2022

Add a preview for the changed file

@PatrickF1
Copy link
Owner

Haven't tried out your code yet but there's already been two PRs for this that I didn't accept (yet) because I want something very specific for it. #138

I want the preview to show both the staged and non-staged changes.

@NextAlone
Copy link
Contributor Author

Haven't tried out your code yet but there's already been two PRs for this that I didn't accept (yet) because I want something very specific for it. #138

I want the preview to show both the staged and non-staged changes.

Just noticed that there were PRs here.

@PatrickF1
Copy link
Owner

Don't let the fact the other two PRs didn't get merged discourage you. It was more because the authors lost interest than this feature being impossible to work on (I hope).
If you can make the preview show both the staged and working directory diffs against HEAD, then I am happy to merge.
Two potential solutions:

@NextAlone
Copy link
Contributor Author

Don't let the fact the other two PRs didn't get merged discourage you. It was more because the authors lost interest than this feature being impossible to work on (I hope).
If you can make the preview show both the staged and working directory diffs against HEAD, then I am happy to merge.
Two potential solutions:

Ok, i will check these two.

@NextAlone
Copy link
Contributor Author

Don't let the fact the other two PRs didn't get merged discourage you. It was more because the authors lost interest than this feature being impossible to work on (I hope). If you can make the preview show both the staged and working directory diffs against HEAD, then I am happy to merge. Two potential solutions:

Are you talking about being displayed together or separately?

@NextAlone
Copy link
Contributor Author

NextAlone commented May 11, 2022

How about this?

It preview untracked files/dirs as ordinary files/dirs, and diff new file with /dev/null, the others diff with HEAD.

if string match -r '\?\?' {1} > /dev/null
    _fzf_preview_file {2..}
else if string match -r 'A' {1} > /dev/null
    git diff --color=always -- /dev/null {2..}
else
    git diff --color=always HEAD -- {2..}
end

Or split cached and uncached changes.

...
else 
    echo 'Unstaged'
    git diff --color=always HEAD -- {2..}
    echo -e '\nStaged'
    git diff --color=always --cached HEAD -- {2..}
end

@PatrickF1
Copy link
Owner

Are you talking about being displayed together or separately?

If there are both staged and working tree changes, I want the preview to show both.

Yeah the second one is along the lines of what I'm thinking. I guess you don't need the intermediate file to store them huh?

@NextAlone
Copy link
Contributor Author

Are you talking about being displayed together or separately?

If there are both staged and working tree changes, I want the preview to show both.

Yeah the second one is along the lines of what I'm thinking. I guess you don't need the intermediate file to store them huh?

Yes, no intermediate file, and add check with status (just simply string match, don't know other convenient way)?

if string match -r '\?\?' {1} > /dev/null
    _fzf_preview_file {2..}
else if string match -r 'A' {1} > /dev/null
    git diff --color=always -- /dev/null {2..}
else 
    if string match -r '\S\s\S' > /dev/null
        echo -e (set_color red)Unstaged
        echo
        git diff --color=always HEAD -- {2..}
        echo
    end
    if string match -r '\s\s|\S\S\s' > /dev/null
        echo (set_color green)Staged
        echo
        git diff --color=always --cached HEAD -- {2..}
    end
end

image

@NextAlone
Copy link
Contributor Author

 X          Y     Meaning
 -------------------------------------------------
          [AMD]   not updated
 M        [ MTD]  updated in index
 T        [ MTD]  type changed in index
 A        [ MTD]  added to index
 D                deleted from index
 R        [ MTD]  renamed in index
 C        [ MTD]  copied in index
 [MTARC]          index and work tree matches
 [ MTARC]    M    work tree changed since index
 [ MTARC]    T    type changed in work tree since index
 [ MTARC]    D    deleted in work tree
             R    renamed in work tree
             C    copied in work tree
 -------------------------------------------------
 D           D    unmerged, both deleted
 A           U    unmerged, added by us
 U           D    unmerged, deleted by them
 U           A    unmerged, added by them
 D           U    unmerged, deleted by us
 A           A    unmerged, both added
 U           U    unmerged, both modified
 -------------------------------------------------
 ?           ?    untracked
 !           !    ignored
 -------------------------------------------------

Not works with R(renamed), C(copied), T(type changed), and unmerged status. i will find out the way.

@NextAlone
Copy link
Contributor Author

Are you talking about being displayed together or separately?

If there are both staged and working tree changes, I want the preview to show both.
Yeah the second one is along the lines of what I'm thinking. I guess you don't need the intermediate file to store them huh?

Yes, no intermediate file, and add check with status (just simply string match, don't know other convenient way)?

if string match -r '\?\?' {1} > /dev/null
    _fzf_preview_file {2..}
else if string match -r 'A' {1} > /dev/null
    git diff --color=always -- /dev/null {2..}
else 
    if string match -r '\S\s\S' > /dev/null
        echo -e (set_color red)Unstaged
        echo
        git diff --color=always HEAD -- {2..}
        echo
    end
    if string match -r '\s\s|\S\S\s' > /dev/null
        echo (set_color green)Staged
        echo
        git diff --color=always --cached HEAD -- {2..}
    end
end

image

maybe this is enough to preview

@NextAlone NextAlone requested a review from PatrickF1 May 13, 2022 21:44
Copy link
Owner

@PatrickF1 PatrickF1 left a comment

Choose a reason for hiding this comment

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

thanks for the great work so far!

git diff --color=always -- $path
echo
end
if string match -r '\S\s\s\S|\S\S\s\S' $argv >/dev/null
Copy link
Owner

Choose a reason for hiding this comment

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

sorry I know you tried to explain what you were doing but I'm still confused about this regex and the one above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe ^\S also works.

@NextAlone
Copy link
Contributor Author

No need to split ^A, it works with staged file

@PatrickF1
Copy link
Owner

Thanks for the updates. This week is really busy so it'll be a while before I can come back to review but just letting you know I won't forget about this

@NextAlone
Copy link
Contributor Author

Thanks for the updates. This week is really busy so it'll be a while before I can come back to review but just letting you know I won't forget about this

It doesn't matter, I will always be here.

1 similar comment
@NextAlone
Copy link
Contributor Author

Thanks for the updates. This week is really busy so it'll be a while before I can come back to review but just letting you know I won't forget about this

It doesn't matter, I will always be here.

@PatrickF1 PatrickF1 changed the title feat: add git status preview [Search git status] preview changed files May 22, 2022
@PatrickF1
Copy link
Owner

Don't worry I haven't forgotten about this, just thinking through what is the best way to make this feature simple and easy to use!

@@ -0,0 +1,20 @@
function _fzf_preview_changed_file
set -l path (string split ' ' $argv)[-1]
if string match -r '^\?\?' $argv --quiet
Copy link
Owner

Choose a reason for hiding this comment

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

Hi, I'm back! I've found a great way to extract the symbols without using regex--you can use git status --porcelain. Can you update the code to use that instead? Afterwards, we'll be very close to merging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't quite understand it and didn't find its use effect

image

Copy link
Owner

@PatrickF1 PatrickF1 Jun 7, 2022

Choose a reason for hiding this comment

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

Try using version 1 porcelain and qualifying the file path. The status it displays is much easier to parse because it's at the very beginning of the line. Then you can use substring instead of playing with unreadable regexes.
Screen Shot 2022-06-06 at 10 45 55 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but it still needs to regex with prefix to detect file status

Copy link
Owner

Choose a reason for hiding this comment

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

What I'd strongly prefer to see is something like this pattern

set file_status (command to grab the two letters of the file status)
if test file_status[0] = "A"

I think this'll be much more readable. Right now I still don't exactly know what your regexes are matching.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I'd strongly prefer to see is something like this pattern

set file_status (command to grab the two letters of the file status)
if test file_status[0] = "A"

I think this'll be much more readable. Right now I still don't exactly know what your regexes are matching.

--porcelain no diff to -s
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I'd strongly prefer to see is something like this pattern

set file_status (command to grab the two letters of the file status)
if test file_status[0] = "A"

I think this'll be much more readable. Right now I still don't exactly know what your regexes are matching.

git status shows with two character, when first not null, it means staged changes, while second, the unstaged.

So, \S\s\S means unstaged and ^\S means staged

Copy link
Owner

Choose a reason for hiding this comment

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

Got it, I get it now, thanks! I am fine with the logic as it is then.

@NextAlone
Copy link
Contributor Author

And if there any other changes request?

@PatrickF1
Copy link
Owner

Nope, I'll take it from here and merge soon :)

@PatrickF1 PatrickF1 merged commit 6d8e962 into PatrickF1:main Jun 13, 2022
@PatrickF1
Copy link
Owner

Thanks for working on this and especially our patience @NextAlone! Really appreciate it. You are the third person to try to PR this change in and it seems third time's the charm. Sorry I've been so busy.

@NextAlone
Copy link
Contributor Author

Thanks for working on this and especially our patience @NextAlone! Really appreciate it. You are the third person to try to PR this change in and it seems third time's the charm. Sorry I've been so busy.

Also thanks for your work, this is such a wonderful plugin.

1 similar comment
@NextAlone
Copy link
Contributor Author

Thanks for working on this and especially our patience @NextAlone! Really appreciate it. You are the third person to try to PR this change in and it seems third time's the charm. Sorry I've been so busy.

Also thanks for your work, this is such a wonderful plugin.

@davidnanry
Copy link

Thank you both! This is an excellent addition to an excellent plugin

@NextAlone
Copy link
Contributor Author

@PatrickF1 And it is time to update Modified paths section in Readme.

@PatrickF1
Copy link
Owner

I did update it. I'll add a gif later. Or did you mean something else?

@NextAlone
Copy link
Contributor Author

I did update it. I'll add a gif later. Or did you mean something else?

Just the gif,thanks for your work.

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.

3 participants