expand paths more selectively

[jamestrew]

There's a few areas where I believe we're over expanding paths.

We've done some quick fixes with vim.fn.escape here and there but I want to explore rather than escaping certain characters then expanding, only expanding when the provided "path" is worth expanding (starts with either '%', '#' or '<').

The places where I'm made changes, it should be safe to assume that at that point, a $ in a path mean the literal $ rather than some shell variable and I'm not even sure if they even ever need expanding.

Edit: ^ this should largely not be a concern anymore. Unlike with vim.fn.escape, vim.fs.normalize only expands $ if it's actually a shell variable.

closes nvim-telescope/telescope-file-browser.nvim#289

also relevant:
#2457

[Conni2461]

I'd like to test run this for a couple of days because it touches some code that i never wanted to touch again (because also windows is affected, and i currently no longer have a easy way to test, without setting up a vm) ^^

Also i dont know if #2457 (#2446) is not fixed by this, because i think they also had trouble opening these files, and action/set.lua isn't touched by this PR

[jamestrew]

For sure.

As for #2446, I'm pretty sure the issue goes beyond telescope based on some of the discussion + some basic testing I did on a windows machine. This PR is related to #2457 in the sense that it deals with avoiding strange expansions.

[Conni2461]

#2457 was never intended to be merged, i just needed that so a windows guy could test out a theory ^^ it should be closed

[AvarianKnight]

This fixes my issue with a few tweaks

make_entry.lua Line 152 and 313 need this check too, afterwards I can search through files with brackets as the path

[Conni2461]

I've just rebased this branch, and am actually testing this now for 2 days straight, maybe we can get this merge then afterwards and get it in 0.1.x prior to 0.1.6

btw james i've wrote you on matrix, regarding that release :)

[Conni2461]

Also also maybe we can see if we need @AvarianKnight remarks in make_entry.lua, can you look at it james?

Avarian do you use windows? if yes maybe you could test the branch with the paths mentioned in this issue: #2446

[AvarianKnight]

Avarian do you use windows? if yes maybe you could test the branch with the paths mentioned in this issue: #2446

On Windows this is still broken, Live Grep shows nothing and opening the file results it opening the wrong path src\routes(app)\loading\+page.svelte

In buffer_previewer.lua:265 the path needs to have a conditional expand

In set.lua:200, it doesn't seem like vim.fn.fnameescape escapes parenthesis.
Escaping the opening and closing parenthesis makes this work fine

filename = vim.fn.escape(Path:new(filename):normalize(vim.loop.cwd()), "()")

After doing these two thing every seems to work fine (though I applied conditional path expands in a few other areas trying to find where the issue was, so this could also be from that)

[jamestrew]

Let me look into this some more. This is actually at least my second attempt trying to use less vim.fn.expand but previous attempt(s) were too aggressive and broke things the other way by under-expanding.

Main issue is stemming from the fact that people can pass non-absolute paths to the cwd option for pickers. Things like ~/ and $XDG_CONFIG_HOME/ need expanding.

If we abandon the goal of porting this fix to telescope 0.1.x, we can use vim.fs.normalize to account for this. I guess we can also use plenary path's :expand() method on a path to remain telescope 0.1.x compatible. But I think plenary's Path tend to be pretty expensive - I can do some light benchmarking to see the difference/effect.

[Conni2461]

yeah lets leave this out, we can always just do multiple releases back to back :D i would personally prefer a solution also for 0.1.x and a solution that doesnt require plenary.path because we kinda wanna move away from that see #2552 😆

Also also i am not sure how stable plenary :expand actually is

[jamestrew]

I went with the lazy approach of just borrowing the existing vim.fs.normalize function into telescope 😅 and then replacing most vim.fn.expand references with our own custom expand function.

I'll do some testing on this branch over the next few days.

[jamestrew]

I'm going to let this fly.