search

martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 260 forks source link

lua: refactor complete-filename plugin #1148

Closed rnpnr closed 7 months ago

rnpnr commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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 commented 7 months ago

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 ~.

mcepl commented 7 months ago

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 commented 7 months ago

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.