fzf-lua icon indicating copy to clipboard operation
fzf-lua copied to clipboard

Windows Support [WIP]

Open TheLeoP opened this issue 1 year ago • 7 comments

This intended to continue the work of #961.

TODO:

  • [ ] Why does Fzflua buffers not work with ctrl-n and ctrl-p?
  • [ ] Add support for quickstart on Windows
  • [ ] Check if shell is cmd.exe, not only if os is windows (support posix shells on windows?)
  • [ ] ???

TheLeoP avatar Jan 15 '24 03:01 TheLeoP

@ibhagwan. Is there something I can help you with right now? If no, I can continue checking that all Fzflua comands are working as expected.

All the changes seem great, I'll check the test on my Linux environment because, last time I checked, PlenaryBusted did not have support for windows :c.

TheLeoP avatar Jan 15 '24 03:01 TheLeoP

Is there something I can help you with right now? If no, I can continue checking that all Fzflua comands are working as expected.

Whatever you can think of, certainly you can let me know which commands do not work properly and we can add them to the TODO list.

If you're up to up you can also write tests for libuv.shellescape, just recently I found out a flaw in the logic whereas if the path ended in \ the resulting escaped string would be invalid as explained in the comment below: https://github.com/ibhagwan/fzf-lua/blob/4e01b2141a01b4e7eae5565442f3bbe5485e48ce/lua/fzf-lua/libuv.lua#L545-L549

last time I checked, https://github.com/nvim-lua/plenary.nvim/issues/255 :c.

Pleanary busted works on my windows VM: image

https://github.com/ibhagwan/fzf-lua/commit/4e01b2141a01b4e7eae5565442f3bbe5485e48ce - latest commit a few minutes ago added support for listing files with the dir command (so we're not dependent on having rg|fd installed) and also fixed path_shorten (the command I ran is in the bottom of the screenshot): image

ibhagwan avatar Jan 15 '24 04:01 ibhagwan

If you're up to up you can also write tests for libuv.shellescape, just recently I found out a flaw in the logic whereas if the path ended in \ the resulting escaped string would be invalid as explained in the comment below:

Would it be ok to allways use / for paths on commands? Or would it better to add a pair of additional \ if a path ends witt \?

Pleanary busted works on my windows VM:

Oh, you are right, I just tested it :o. Those are awesome news.

TheLeoP avatar Jan 15 '24 04:01 TheLeoP

Would it be ok to allways use / for paths on commands? Or would it better to add a pair of additional \ if a path ends witt \?

Not sure if I understand your question properly, I think it would be ok to use / for all paths that are hidden from the user (is that what you mean by "commands"?) but if the user specifically requested say :FzfLua files cwd=c:\... I'd still prefer to display the paths unchanged from the users' request.

ibhagwan avatar Jan 15 '24 04:01 ibhagwan

If you take a look at tests\path_spec.lua you'll see I have all kinds of tests for different separator combos, the new path module logic considers both \ and / as valid separators on windows (even mixed and matched inside one path e.g. c:\some/path) and also semi-heuristics do determine which separator style to add if one is needed: https://github.com/ibhagwan/fzf-lua/blob/4e01b2141a01b4e7eae5565442f3bbe5485e48ce/lua/fzf-lua/path.lua#L14-L25

Edit: Btw, I haven't finished the rewrite of the path module, I still wish to revamp entry_to_file.

ibhagwan avatar Jan 15 '24 04:01 ibhagwan

@TheLeoP, shouldn't change much as you didn't add new commits yet, just FYI that I rebased the windows branch again after the recent commits.

ibhagwan avatar Jan 15 '24 23:01 ibhagwan

@TheLeoP, I know I've said this beore but I think I've nailed it, the perfect windows escape logic for CMD.exe which is a combined approach of blackslahes and carets.

If you want you can get a glimpse of windows going full retard at tests\libuv_spec.lua: https://github.com/ibhagwan/fzf-lua/blob/7b9069f3cc5f9e1d7f3c4944a4407830fc7fb529/tests/libuv_spec.lua#L29-L76

Once libuv.shellescape and the path module was figured it seems to smooth sailing as not too many adjustments need to be done, other than extra dumb windows suff such as having to double the escape sequences if ! is found anywhere in the command: https://github.com/ibhagwan/fzf-lua/blob/7b9069f3cc5f9e1d7f3c4944a4407830fc7fb529/lua/fzf-lua/make_entry.lua#L381-L388

I'm a bit tired so I didn't test everything fully (which I intend to invest time in tomorrow) but I think it's fair to say I'm getting close to finalizing the windows code which I also utilized for general improvements (base64 encoding of the arguments, code dedup where possible, etc).

My plan is to make a list of pickers and options to test and make sure these work on Mac/Linux/Windows, e.g.:

  • Files
    • cwd
    • cwd_prompt
    • previewer: builtin
    • previewer: bat
    • dir (no rg/fd installed)
    • rg (no fd installed)
    • fd
    • file_ignore_patterns
    • multiprocess (true/false)

Would be appreicated if you can do some rudimentary testing and lmk if the code is on the right track and works on your environment as it works on mine?

ibhagwan avatar Jan 28 '24 04:01 ibhagwan

This PR is mainly used for tracking, since I'm getitng closer to releasing the windows branch for beta testing I'm going to close this PR and continue tracking in #1038.

Once fully merged your original contribution from #961 will retain it's authorship and be merged to main under your name.

ibhagwan avatar Feb 12 '24 00:02 ibhagwan