denisidoro / navi

An interactive cheatsheet tool for the command-line
Apache License 2.0
15.22k stars 507 forks source link

`--expand` doesn't work with `--map` #708

Open lacygoill opened 2 years ago

lacygoill commented 2 years ago

Describe the bug

The --expand parameter doesn't work when specifying the default values of an argument, if the --map parameter is also used.

To Reproduce

In the file /tmp/cheats/cheat.cheat, write this snippet:

# this should result into: echo "a.txt" "b.txt" "c.txt"
echo <files>
$ files: find . -iname '*.txt' --- --map 'sed "s/z/Z/"' --multi --expand

Next, in an interactive shell, execute these commands:

$ touch {a,b,c}.txt
$ navi --best-match --print --query=result --path=/tmp/cheats/

In the fzf window, select all of the 3 matches: a.txt, b.txt and c.txt. To do so, press CTRL-p twice, then Tab three times. Finally, press Enter.

This is echo'ed:

echo ./c.txt
./b.txt
./a.txt

Expected behavior

This is echo'ed:

echo "./c.txt" "./b.txt" "./a.txt"

Because that's how --expand is documented in docs/cheatsheet_syntax.md:

--expand: (experimental) converts each line into a separate argument

source

# This will result into: cat "file1.json" "file2.json"
cat <jsons>

$ jsons: find . -iname '*.json' -type f -print --- --multi --expand

source

Versions:

Additional context

The issue disappears once we remove the --map parameter when specifying the default values of the files argument:

$ files: find . -iname '*.txt' --- --map 'sed "s/z/Z/"' --multi --expand
                                   ^------------------^
                                            ✘

However, note that in this example, --map executes a sed(1) command which doesn't even alter the results. It executes a substitution command replacing z with Z; but there is no z in a.txt, b.txt, c.txt.

lacygoill commented 2 years ago

Also, the final value of the files argument as displayed in the preview window looks wrong.

Expected:

files = "./c.txt" "./b.txt" "./a.txt"

Actual (with --map):

files = ./c.txt' './b.txt' './a.txt

Actual (without --map):

files = "./c.txt' './b.txt' './a.txt"
lacygoill commented 2 years ago

I guess that under the hood, navi itself uses --map to implement --expand; which would explain why the latter fails when we use --map. We're probably overriding whatever navi intended to do with it.

If that's the case, maybe it should be allowed to specify several --map parameters, and make sure that --expand is executed last (i.e. after all the other --maps).

denisidoro commented 2 years ago

Your hypothesis is correct: https://github.com/denisidoro/navi/blob/3b1038c8849d5b9d4a15452646f74dc10a435b50/src/parser.rs#L37

The solution would be to make map and expand independent of each other