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

fix(from_entry): escape paths with $ symbol #2412

Merged

Conversation

jamestrew
Copy link
Contributor

@jamestrew jamestrew commented Mar 2, 2023

Description

Adds support for viewing File Previews of paths with dollar symbols.
Eg. ./($lang)._index.tsx
This is a new pattern supported by Remix optional segments file based routing.

Partially fixes downstream issue nvim-telescope/telescope-file-browser.nvim#244.
This fix also applies to many of the builtin pickers.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list relevant details about your configuration

  • Test A
  1. Create a file touch "(\$lang)._index.ts"
  2. In that created file, enter some text console.log('hello');
  3. With and without the PR checked out, use :Telescope find_files and search of the file and at look the File Preview
  4. Without the PR is a blank preview; with shows the contents of the file
  • Test B
  1. Verify other files can still be previewed find

Configuration:

  • Neovim version (nvim --version):
NVIM v0.9.0-dev-1070+g2630341db
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
  • Operating system and version:
    Linux archlinux 6.1.12-arch1-1 #1 SMP PREEMPT_DYNAMIC Tue, 14 Feb 2023 22:08:08 +0000 x86_64 GNU/Linux

Checklist:

  • My code follows the style guidelines of this project (stylua)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (lua annotations)

@jamestrew
Copy link
Contributor Author

jamestrew commented Mar 2, 2023

I also want to discuss implementing the same change to this bit of code here:

self.uv_opts.cwd = vim.fn.expand(opts.cwd)

The lack of character escaping is impacting telescope-file-browser's file navigation. More details in the linked issue above but happy to provide more contest/summary.

Also if Conni, or if there's any other plenary maintainers seeing this, I'd like to discuss implementing the same change here: https://github.com/nvim-lua/plenary.nvim/blob/253d34830709d690f013daf2853a9d21ad7accab/lua/plenary/job.lua#L68
This will support running jobs using cwd such as ./($lang)/. This is also part of the issue linked above.

@jamestrew jamestrew changed the title fix(from_entry): escape paths from $ symbol fix(from_entry): escape paths with $ symbol Mar 2, 2023
Adds support for viewing File Previews of paths with dollar symbols.
@jamestrew jamestrew force-pushed the fix/escape-dollar-sign-path branch from 111a611 to 9c79225 Compare March 2, 2023 04:51
@juanpprieto
Copy link

Can confirm that telescope seems to not like paths containing ( followed by $ and also with a nested folder prefixed with __ such as /app/($lang)/__public/xyz.

I noticed a few extensions choking when working with files on these paths. FWIW, this is a common file-based routing naming convention for frameworks such as NextJS and Remix.

🙏🏼

@tjdevries
Copy link
Member

@jamestrew if there's something you'd rather do in plenary to make this easy / universally useful, then we could totally do that. What would you like to do instead if we moved this into plenary?

@tjdevries
Copy link
Member

Also, doesn't this then break doing things like $HOME ?

@jamestrew
Copy link
Contributor Author

@jamestrew if there's something you'd rather do in plenary to make this easy / universally useful, then we could totally do that. What would you like to do instead if we moved this into plenary?

For this PR, making this change within telescope makes sense to me since it's more to do with how the previewer takes and handles the entry_maker's entries.

I would ultimately like to do some similar character escaping in these locations:

But I wanted to get this PR through below I go around making similar changes to other areas.



Also, doesn't this then break doing things like $HOME ?

Do you mean having files/folder with the literal string $HOME (ie touch "\$HOME, mkdir "\$HOME"). That would be handle just fine. If you mean $HOME expanded, I don't think you can have that.

@Conni2461
Copy link
Member

thanks :)

Conni2461 pushed a commit that referenced this pull request Apr 9, 2023
Adds support for viewing File Previews of paths with dollar symbols.

(cherry picked from commit 7141515)
abelmul pushed a commit to abelmul/telescope.nvim that referenced this pull request Jun 6, 2023
Adds support for viewing File Previews of paths with dollar symbols.
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.

None yet

4 participants