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

lua: refactor complete-filename plugin #1148

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Conversation

rnpnr
Copy link
Collaborator

@rnpnr rnpnr commented Oct 13, 2023

There are probably more things to simplify but at least this makes it easier to see what exactly is different between <C-x><C-f> and <C-x><C-o>.

closes #1146

Comments on how to simplify this more would be appreciated!

@mcepl
Copy link
Contributor

mcepl commented Oct 13, 2023

I have now rebased that patch by João on the top of this PR as https://git.sr.ht/~mcepl/vis/commit/09e25541cace . I think it is sufficiently obvious and simple thing, which can only help. If not here, then I will probably send it to the list, when you merge this PR.

@rnpnr rnpnr force-pushed the filename branch 3 times, most recently from a56e623 to 57e9da9 Compare October 17, 2023 18:28
@rnpnr
Copy link
Collaborator Author

rnpnr commented Oct 17, 2023

I made a couple changes to remove some differences between the two modes:

  • whitespace in range is treated the same for both actions
  • empty range will expand to files in PWD for both actions

For the second point vis-complete has some edge case that causes the name of the current directory to be added if invoked as vis-complete --file '' but that can be fixed separately.

@rnpnr
Copy link
Collaborator Author

rnpnr commented Oct 17, 2023

I have now rebased that patch by João on the top of this PR as https://git.sr.ht/~mcepl/vis/commit/09e25541cace

Thinking about it again this doesn't belong in complete-filename.lua . It should be made in vis-open just like how vis-complete handles ~ separately.

@mcepl
Copy link
Contributor

mcepl commented Oct 17, 2023

I have now rebased that patch by João on the top of this PR as https://git.sr.ht/~mcepl/vis/commit/09e25541cace

Thinking about it again this doesn't belong in complete-filename.lua . It should be made in vis-open just like how vis-complete handles ~ separately.

Looking at the code:

        local cmdfmt = "vis-complete --file '%s'"
 	if expand then cmdfmt = "vis-open -- '%s'*" end

Shouldn’t be fixed in both?

@rnpnr
Copy link
Collaborator Author

rnpnr commented Oct 17, 2023

Shouldn’t be fixed in both?

Sorry I mean in the shell scripts themselves. vis-complete already handles it. It just needs to be added to vis-open.

@mcepl
Copy link
Contributor

mcepl commented Nov 2, 2023

Shouldn’t be fixed in both?

Sorry I mean in the shell scripts themselves. vis-complete already handles it. It just needs to be added to vis-open.

Actually, can you reproduce it? Even without João’s commit, vis-open ~/.bashrc does what it should?

@rnpnr
Copy link
Collaborator Author

rnpnr commented Nov 3, 2023

Actually, can you reproduce it?

Yes, when invoked inside vis via <C-x><C-o> because ~ isn't expanded before being passed to vis-open. I see the argument for having it be a part of complete-filename.lua but in that case the special handling should be removed from vis-filename vis-complete.

I think I will just apply this since it cleans up the docs and simplifies a few things and leave it up to someone else to propose a change for expanding ~.

There are probably more things to simplify but at least this makes
it easier to see what exactly is different between `<C-x><C-f>` and
`<C-x><C-o>`.

Some differences were removed:
* whitespace in range is treated the same for both actions
* empty range will expand to files in CWD for both actions

closes martanne#1146: Complete file name and file path swapped in doc
@rnpnr rnpnr merged commit 1e64b1c into martanne:master Nov 3, 2023
@rnpnr rnpnr deleted the filename branch November 3, 2023 15:56
@mcepl
Copy link
Contributor

mcepl commented Nov 5, 2023

because ~ isn't expanded before being passed to vis-open

And why should it matter when I have shown that vis-open ~/filename works as expected?

@rnpnr
Copy link
Collaborator Author

rnpnr commented Nov 5, 2023

why should it matter when I have shown that vis-open ~/filename works as expected?

It doesn't if invoked inside vis. Open a new file and do i~/<C-x><C-f> and it will expand to your home directory. Now do i~/<C-x><C-o> and nothing will happen.

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.

Complete file name and file path swapped in doc
2 participants