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

feat: add a keybind to open current file in editor #313

Merged
merged 3 commits into from
Jun 19, 2023
Merged

Conversation

wfxr
Copy link
Owner

@wfxr wfxr commented Jun 14, 2023

Check list

  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation

Description

Add a keybind alt-e to allow open current file in default editor. This will facilitate doing diff and modify loop.

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation change

Test environment

  • Shell
    • bash
    • zsh
    • fish
  • OS
    • Linux
    • Mac OS X
    • Windows
    • Others:

Signed-off-by: Wenxuan Zhang <wenxuangm@gmail.com>
@wfxr wfxr changed the title feat: add keybind to open current file in editor feat: add a keybind to open current file in editor Jun 14, 2023
Copy link
Collaborator

@sandr01d sandr01d left a comment

Choose a reason for hiding this comment

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

Great idea, this could massively improve my workflow! I've requested a minor improvement though.

bin/git-forgit Outdated Show resolved Hide resolved
bin/git-forgit Outdated Show resolved Hide resolved
Co-authored-by: sandr01d <88739791+sandr01d@users.noreply.github.com>
@wfxr
Copy link
Owner Author

wfxr commented Jun 15, 2023

Great idea, this could massively improve my workflow! I've requested a minor improvement though.

Done.

@wfxr wfxr requested a review from sandr01d June 15, 2023 01:50
@carlfriedrich
Copy link
Collaborator

@wfxr Great addition, works for me!

@sandr01d Great idea as well! However, refreshing does not work correctly for me. For some reason, the fzf screen is not fully redrawn after quitting the editor, so that I get a mixture of shell promts and fzf. Any idea why this might be the case?

And some further idea: consider you edit the file and revert all the changes, then the file should not appear in the fzf dialog at all afterwards. Maybe this can be achieved with the reload() action of fzf? Haven't tried it, because I do not have time right now. But maybe someone of you might check that?

@wfxr
Copy link
Owner Author

wfxr commented Jun 15, 2023

@sandr01d Great idea as well! However, refreshing does not work correctly for me. For some reason, the fzf screen is not fully redrawn after quitting the editor, so that I get a mixture of shell promts and fzf. Any idea why this might be the case?

@carlfriedrich Press CTRL-L to flush the buffer usually works when I sometimes have a similar problem when resizing the window.

And some further idea: consider you edit the file and revert all the changes, then the file should not appear in the fzf dialog at all afterwards. Maybe this can be achieved with the reload() action of fzf? Haven't tried it, because I do not have time right now. But maybe someone of you might check that?

I tried and couldn't find a way to get reload() action to work for this purpose. Maybe we can improve it in a future pr?

EDIT

I have changed my mind, and tend to keep the file list as fzf started, even if changes on a specified file are reverted. This gives us the opportunity to select and edit it again easily. So IMO @sandr01d's refresh-preview improvement is good enough. What do you think?

@sandr01d
Copy link
Collaborator

@sandr01d Great idea as well! However, refreshing does not work correctly for me. For some reason, the fzf screen is not fully redrawn after quitting the editor, so that I get a mixture of shell promts and fzf. Any idea why this might be the case?

@carlfriedrich I can not reproduce this. Does this happen only with the refresh-preview or also without it?

I have changed my mind, and tend to keep the file list as fzf started, even if changes on a specified file are reverted. This gives us the opportunity to select and edit it again easily. So IMO @sandr01d's refresh-preview improvement is good enough. What do you think?

I definitively see the use case here. Might be a bit confusing for some users though. Maybe we could be transparent about it and indicate that the file has no changes anymore in the preview, similar as we do here:

https://github.com/wfxr/forgit/blob/ec1bd214683d1244f2512222dee11ac467e4ed17/bin/git-forgit#LL588C1-L594C6

@carlfriedrich
Copy link
Collaborator

@carlfriedrich I can not reproduce this. Does this happen only with the refresh-preview or also without it?

@sandr01d It actually happens with and without it, so adding it is good in either way.

@carlfriedrich Press CTRL-L to flush the buffer usually works when I sometimes have a similar problem when resizing the window.

@wfxr That works indeed, thanks for the hint!

I have changed my mind, and tend to keep the file list as fzf started, even if changes on a specified file are reverted. This gives us the opportunity to select and edit it again easily. So IMO @sandr01d's refresh-preview improvement is good enough. What do you think?

I see your point and agree with you.

bin/git-forgit Outdated Show resolved Hide resolved
Signed-off-by: Wenxuan Zhang <wenxuangm@gmail.com>
Copy link
Collaborator

@carlfriedrich carlfriedrich left a comment

Choose a reason for hiding this comment

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

@wfxr Great, approved!

@wfxr wfxr merged commit c78c10a into master Jun 19, 2023
@wfxr wfxr deleted the feat/open_in_editor branch June 19, 2023 01:26
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