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

Implement change picker #7990

Closed
wants to merge 3 commits into from
Closed

Conversation

dgyurov
Copy link
Contributor

@dgyurov dgyurov commented Aug 18, 2023

What is this Pull Request about?

This pull request is introducing a new picker, which displays a list of all current changes in the current buffer.

How can I try it?

The new picker can be accessed in space mode, by pressing c.

How does it work?

  1. If there are changes in the current file a picker with preview of the location where the selected hunk is will be displayed.
  2. If there are no changes in the current file, the picker will not be displayed and a status message will be displayed instead.

Visual preview

Adds a new entry underneath the workspace diagnostics picker

1692350940

The change picker in action

1692350891

Considerations

  1. It will be really nice if we also have a workspace change picker. It will display a full overview of all changes in the current space. This can be bound to <space>+C for example. I think such features should be part of a future pull request.
  2. In order to be consistent with the ]g and [g motions, it would have been better to use <space>+g for the change picker, however that is currently occupied by the debug menu and I'm not sure if it is a good idea to change an existing default binding.

@woojiq
Copy link
Contributor

woojiq commented Aug 18, 2023

Duplicate of #5472

@pascalkuthe
Copy link
Member

It will be really nice if we also have a workspace change picker. It will display a full overview of all changes in the current space. This can be bound to +C for example. I think such features should be part of a future pull request.

This would depend on #5645 which depends on upstream gitoxide changes.

The way I think ablut it is that #5645 is the equivalent of git status while this PR would be the equivalent of git diff. Just like I mentioned in #5472 I am not sure that a picker is the right format for git diff. I am wokring towards adding a diff mode and conceal to helix. In way to have a much nicer git diff display would be to hide all unchanged lines and only show the changed lines (with the original files shown as changed too) with a background highlight.

I an not quite sure I an a huge fan of a picker like this because it doesn't really use the fuzzy matching of the picker.

The only thing to fuzzy match on is the lines itself. That is similar to #3462. So if you don't just want to see a git diff like view (which I think is better realized as described above) but instead want to.fuzzy match on changed limes than it's breter to wait for #7265 and then implement #3462 with an extra column for diff status (changed, added).

@dgyurov
Copy link
Contributor Author

dgyurov commented Aug 18, 2023

It will be really nice if we also have a workspace change picker. It will display a full overview of all changes in the current space. This can be bound to +C for example. I think such features should be part of a future pull request.

This would depend on #5645 which depends on upstream gitoxide changes.

The way I think ablut it is that #5645 is the equivalent of git status while this PR would be the equivalent of git diff. Just like I mentioned in #5472 I am not sure that a picker is the right format for git diff. I am wokring towards adding a diff mode and conceal to helix. In way to have a much nicer git diff display would be to hide all unchanged lines and only show the changed lines (with the original files shown as changed too) with a background highlight.

I an not quite sure I an a huge fan of a picker like this because it doesn't really use the fuzzy matching of the picker.

The only thing to fuzzy match on is the lines itself. That is similar to #3462. So if you don't just want to see a git diff like view (which I think is better realized as described above) but instead want to.fuzzy match on changed limes than it's breter to wait for #7265 and then implement #3462 with an extra column for diff status (changed, added).

Thank you for the quick feedback! #5645 looks really great and I think it (in combination with the existing motions) will obsolete the implementation provided by this PR. I'll close my PR for now and rethink the design of this feature. I'm not certain that a non interactable git diff like functionality will add much value to Helix at the moment. Thank you again! :)

@dgyurov dgyurov closed this Aug 18, 2023
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