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

[QUESTION] How do I escape asterisk (*) when typing a command or creating a keybinding #744

Open
Yutsuten opened this issue Nov 27, 2023 · 6 comments

Comments

@Yutsuten
Copy link
Contributor

Greetings.

I'm trying to set my wallpaper in sway using the image I'm currently viewing in vimiv. The command is !swaymsg output * bg % fill, but I noticed that the * is being expanded to all files in the current directory.

I tried:

  • !swaymsg output '*' bg % fill
  • !swaymsg output "*" bg % fill
  • !swaymsg output \* bg % fill

I have no idea what else I could try. In fish I can just use single quotations and it works.

I also tried using spawn, but it always say something on the lines /usr/bin/fish: option '--command' requires an argument.
Here some of my attempts:

  • spawn swaymsg output '*' bg % fill
  • spawn --shell=fish --shellarg=--command swaymsg output '*' bg % fill
  • spawn swaymsg output '*' bg % fill --shell=fish --shellarg=--command
@karlch
Copy link
Owner

karlch commented Nov 27, 2023

Thanks for your bug report! Upon a first glance, I can reproduce this, but am unsure what is happening behind the scenes. I will try to take a detailed look tonight after work 😊

karlch added a commit that referenced this issue Nov 27, 2023
This has various issues, and no trivial fix. We can rely on the shell
with spawn for globbing, e.g.,
:spawn "!rm Image*.jpg"
as long as the shell supports * and friends.

See #744 and #652 for some more details.
@karlch
Copy link
Owner

karlch commented Nov 27, 2023

So, this is very much intertwined with the wildcard havoc discussed in #652. I have pushed a few more commits to the related fix-mark-special-chars branch, which deactivates globbing for external commands and clarifies a lot of the issues. This should fix the bug you have also noticed.

However, this is still not complete as the completion (and related commandline escaping) for paths which contain glob chars (*[]) is still not correct. I will try to finalise this sooner rather than later, but no promises unfortunately.

@Yutsuten
Copy link
Contributor Author

The issue seems to be much more complex than I was thinking.
Here's my opinion:

External commands starting with ! should not support globbing. There is a note in the docs that says:

External commands started with ! do not run in a sub-shell for security and performance reasons. This means that redirection with | or > as well as any other shell specifics do not work. If you require to run with a sub-shell, use the :spawn command instead.

From my perspective, globbing is a shell feature. Having shell-like features on an external command ! is confusing. One may wrongly think it is a shell. Having globbing on "normal" vimiv commands is ok tho. It is a vimiv command feature.

If one wants globbing feature while running external commands, :spawn should pass the command as-is to the shell the user provided. So something like spawn swaymsg output '*' bg % fill --shell=fish --shellarg=--command should work (haven't tested the new branch yet, tho). A vimiv's native glob expanding feature here is not desired at all.

About the another special characters (~ [] ?), I think there are two ways to solve:

  • Completely disable those features on certain commands
  • Automatically escape (if there is a way to) those characters

On problems like this, I like to see how well established tools like fish perform on the task. I created the following files here now:

> touch 'This*is*a*file.txt'
> touch 'This?is[a]file.txt'
> ls -l
合計 0
-rw-r--r-- 1 mateus mateus 0 11月 28 11:04 This*is*a*file.txt
-rw-r--r-- 1 mateus mateus 0 11月 28 11:04 This?is[a]file.txt

Then I expand this using glob. First I type:

ls -l *.txt

Then press TAB. It becomes:

ls -l This\*is\*a\*file.txt This\?is\[a\]file.txt

So instead of reinventing the wheel, the ideal is to imitate what the shell do. The shell developers already solved the problem, this is probably the best solution, there is no need to solve the same problem again in another way.

My 2 cents... 😇

@karlch
Copy link
Owner

karlch commented Nov 28, 2023

Thanks for your detailed thoughts! 😊

I tend to agree with you on all points raised (bar the fact that we will of course not have the funky expansion on TAB 😄 ).

The current implementation in the fix-mark-special-chars branch does get close, however there are still corner-cases. Our main issue is the grown havoc of different parts that need (partially different) escaping. This includes, at least, shlex.split, glob.glob, the shell (for :spawn), and internal wildcards. I will need to wrap my head around these and draw out all paths I guess 🤯

This currently leaves at least these two bugs I can see:

  • Passing a path with globchars, e.g., image[a].jpg to external commands has unnecessary \ in case of :!xyz %. This is not an issue for internal commands, as we handle the case explicitly. And this is not an issue for :spawn as the shell will also expect these to be escaped.
  • Commandline completion suggests image*.jpg for a path with exactly this name. This will crash for internal commands, as we will run the glob. Having a single image\*.jpg as the solution we would like does not work as-is, as it will be removed by shlex.split before the globbing, which already treats the \ as an escape character, and removes it.

@Yutsuten
Copy link
Contributor Author

Yutsuten commented Nov 29, 2023

bar the fact that we will of course not have the funky expansion on TAB

Oh yes, it was a reference example, I did not mean to have a new feature 😆

So if I understood correctly:

  • % needs to expand differently depending on context
    • Change how it expands?
    • Do not change how it expands, just process it further?
  • Commandline suggestion must consider escaping
  • Shouldn't stuff like globbing happen before the processing of escapes (shlex.split)? Even though this fix probably is not as simple as changing the order of some code...
    • Or we can workaround shlex.split removing escape characters by systematically replacing \ with \\ (is there any use case for a single escape \ that will be removed anyway?)
    • Answering my own question, \ (escape of a space) is probably correctly handled by shlex.split
    • So systematically replace \* \[ \] \? with \\* \\[ \\] \\? ? 😁

@karlch
Copy link
Owner

karlch commented Nov 30, 2023

This is pretty much it, let me add to your points:

  • Not really, but we do need to consider escaping characters within the path or not, depending on the context.
  • Yes, and also this is different depending on the context.
  • Globbing unfortunately cannot happen before shlex.split as only after the split we know the command (i.e. the first part of the split) and thus the context (internal, external, ...).

I suppose we could figure out the context after the split and then (re-) escape accordingly? Maybe your systematic approach also works. In any case, will need some solid testing and fiddling 😆 Maybe I take the time over the week-end 🤞

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

No branches or pull requests

2 participants