Fails on env vars with quotes

[cdfa]

Hi! First of all, I would like to thank you for this nice tool. I ran into an issue today when using it in a nix shell. This shell sets a variable $shellHook which has quotes (specifically ") in it, which results in replay printing some part of the value of $shellHook.

I didn't manage to reproduce the printing, but there is definitely something wrong with quotes handling:

$ replay 'export TEST=\'echo "hi"\''
$ echo $TEST
echo hi

Quotes around hi are missing. I played around a little myself and this seems to fix the issue:

36c36,37
<                             echo "set --global --export $name \"$value\""
---
>                             set --local tmp (string replace --all \" \\\" $value)
>                             echo "set --global --export $name \"$tmp\""

I would make a PR, but I don't know enough about how replay works to know if there are problems with this or other changes are needed.

[jorgebucaran]

Definitely a bug, thank you for reporting. I think we could solve this using string escape too.

Care you try

echo "set --global --export $name "(string escape -- $value)

and let me know if it works for you?

[cdfa]

That seems to work and it's a lot cleaner too! Thank you :)

[cdfa]

Huh, actually it didn't fix the problem with $shellHook. Let me debug a little further

[cdfa]

I'll just give you the complete contents of $shellHook. Hopefully you can see what is going wrong.

Indended contents:

export PATH=$PATH:/nix/store/dqlx77d224grs7ch39jhgfplf68mg6sl-python3.8-pre-commit-2.7.1/bin
if ! type -t git >/dev/null; then
  # This happens in pure shells, including lorri
  echo 1>&2 "WARNING: nix-pre-commit-hooks: git command not found; skipping installation."
else
  # These update procedures compare before they write, to avoid
  # filesystem churn. This improves performance with watch tools like lorri
  # and prevents installation loops by via lorri.

  if readlink .pre-commit-config.yaml >/dev/null \
    && [[ $(readlink .pre-commit-config.yaml) == /nix/store/vjls5bggmd66rlrid5pjslzyd1qd7xc2-pre-commit-config.json ]]; then
    echo 1>&2 "nix-pre-commit-hooks: hooks up to date"
  else
    echo 1>&2 "nix-pre-commit-hooks: updating $PWD repo"

    [ -L .pre-commit-config.yaml ] && unlink .pre-commit-config.yaml

    if [ -e .pre-commit-config.yaml ]; then
      echo 1>&2 "nix-pre-commit-hooks: WARNING: Refusing to install because of pre-existing .pre-commit-config.yaml"
      echo 1>&2 "    1. Translate .pre-commit-config.yaml contents to the new syntax in your Nix file"
      echo 1>&2 "        see https://github.com/hercules-ci/nix-pre-commit-hooks#getting-started"
      echo 1>&2 "    2. remove .pre-commit-config.yaml"
      echo 1>&2 "    3. add .pre-commit-config.yaml to .gitignore"
    else
      ln -s /nix/store/vjls5bggmd66rlrid5pjslzyd1qd7xc2-pre-commit-config.json .pre-commit-config.yaml
      pre-commit install
    fi
  fi
fi

Log:

replay 'export shellHook=\'export PATH=$PATH:/nix/store/dqlx77d224grs7ch39jhgfplf68mg6sl-python3.8-pre-commit-2.7.1/bin
if ! type -t git >/dev/null; then
# This happens in pure shells, including lorri
echo 1>&2 "WARNING: nix-pre-commit-hooks: git command not found; skipping installation."
else
# These update proceduresexport PATH=$PATH:/nix/store/dqlx77d224grs7ch39jhgfplf68mg6sl-python3.8-
# filesystem churn. This improves performance with watch tools like lorri
# and prevents installation loops by via lorri.

if readlink .pre-commit-config.yaml >/dev/null \\
&& [[ $(readlink .pre-commit-config.yaml) == /nix/store/vjls5bggmd66rlrid5pjslzyd1qd7xc2-pre-commit-config.json ]]; then
echo 1>&2 "nix-pre-commit-hooks: hooks up to date"
else
echo 1>&2 "nix-pre-commit-hooks: updating $PWD repo"
\\
[ -L .pre-commit-config.yaml ] && unlink .pre-commit-config.yaml

if [ -e .pre-commit-config.yaml ]; then
echo 1>&2 "nix-pre-commit-hooks: WARNING: Refusing to install because of pre-existing .pre-commit-config.yaml"
echo 1>&2 "    1. Translate .pre-commit-config.yaml contents to the new syntax in your Nix file"
echo 1>&2 "        see https://github.com/hercules-ci/nix-pre-commit-hooks#getting-started"
echo 1>&2 "    2. remove .pre-commit-config.yaml"
echo 1>&2 "    3. add .pre-commit-config.yaml to .gitignore"
else
ln -s /nix/store/vjls5bggmd66rlrid5pjslzyd1qd7xc2-pre-commit-config.json .pre-commit-config.yaml
pre-commit install
fi
fi
fi\''





\''
- (line 11): Expected a variable name after this $.
\ \ \ \ \&\&\ \[\[\ \\$\(readlink\ .pre-commit-config.yaml\)\ ==\ /nix/store/vjls5bggmd66rlrid5pjslzyd1qd7xc2-pre-commit-config.json\ \]\]\;\ then\
^
from sourcing file -
called on line 24 of file ~/.config/fish/functions/replay.fish
in function 'replay' with arguments 'export\ shellHook=\'export\ PATH=\$PATH:/nix/store/dqlx77d224grs7ch39jhgfplf68mg6sl-python3.8-pre-commit-2.7.1/bin\nif\ !\ type\ -t\ git\ \>/dev/null\;\ then\n\ \ \#\ This\ happens\ in\ pure\ shells,\ including\ lorri\n\ \ echo\ 1\>\&2\ \"WARNING:\ nix-pre-commit-hooks:\ git\ command\ not\ found\;\ skipping\ installation.\"\nelse\n\ \ \#\ These\ update\ procedures\ compare\ before\ they\ write,\ to\ avoid\n\ \ \#\ filesystem\ churn.\ This\ improves\ performance\ with\ watch\ tools\ like\ lorri\n\ \ \#\ and\ prevents\ installation\ loops\ by\ via\ lorri.\n\n\ \ if\ readlink\ .pre-commit-config.yaml\ \>/dev/null\ \\\n\ \ \ \ \&\&\ \[\[\ \$\(readlink\ .pre-commit-config.yaml\)\ ==\ /nix/store/vjls5bggmd66rlrid5pjslzyd1qd7xc2-pre-commit-config.json\ \]\]\;\ then\n\ \ \ \ echo\ 1\>\&2\ \"nix-pre-commit-hooks:\ hooks\ up\ to\ date\"\n\ \else\n\ \ \ \ echo\ 1\>\&2\ \"nix-pre-commit-hooks:\ updating\ \$PWD\ repo\"\n\n\ \ \ \ \[\ -L\ .pre-commit-config.yaml\ \]\ \&\&\ unlink\ .pre-commit-config.yaml\n\n\ \ \ \ if\ \[\ -e\ .pre-commit-config.yaml\ \]\;\ then\n\ \ \ \ \ \ echo\ 1\>\&2\ \"nix-pre-commit-hooks:\ WARNING:\ Refusing\ to\ install\ because\ of\ pre-existing\ .pre-commit-config.yaml\"\n\ \ \ \ \ \ echo\ 1\>\&2\ \"\ \ \ \ 1.\ Translate\ .pre-commit-config.yaml\ contents\ to\ the\ new\ syntax\ in\ your\ Nix\ file\"\n\ \ \ \ \ \ echo\ 1\>\&2\ \"\ \ \ \ \ \ \ \ see\ https://github.com/hercules-ci/nix-pre-commit-hooks\#getting-started\"\n\ \ \ \ \ \ echo\ 1\>\&2\ \"\ \ \ \ 2.\ remove\ .pre-commit-config.yaml\"\n\ \ \ \ \ \ echo\ 1\>\&2\ \"\ \ \ \ 3.\ add\ .pre-commit-config.yaml\ to\ .gitignore\"\n\ \ \ \ else\n\ \ \ \ \ \ ln\ -s\ /nix/store/vjls5bggmd66rlrid5pjslzyd1qd7xc2-pre-commit-config.json\ .pre-commit-config.yaml\n\ \ \ \ \ \ pre-commit\ install\n\ \ \ \ fi\n\ \ fi\nfi\''
source: Error while reading file “-”

[jorgebucaran]

Does it work as expected with the fix you suggested before?

[cdfa]

Yes

[cdfa]

Although variables with 'do form a problem:

$ replay "export MAKEFLAGS='-j 8'"
string replace: Unknown option “-j 8”

~/.config/fish/functions/replay.fish (line 1): 
in command substitution
        called on line 13 of file ~/.config/fish/functions/replay.fish
in command substitution
        called on line 24 of file ~/.config/fish/functions/replay.fish
in function 'replay' with arguments 'export\ MAKEFLAGS=\'-j\ 8\''

(Type 'help string' for related documentation)

[jorgebucaran]

@cdfa Anything?

[cdfa]

Apologies for the slow response. Work has been very busy. I did experiment a little and the version with echo "set --global --export $name "(string escape -- $value) does seem to work with $NIX_SHELL variable as well now. Probably I was using something different when I was testing it before 😅😓. Anyway, the version you suggested should be used for the fix.

[jorgebucaran]

Thank you, @cdfa! Please update to the latest version and try again to make sure everything is fine on your side. 👍

[cdfa]

So I must have messed something up with testing, because it was broken again after the last update. The issue seems to be | string replace --all -- \$ \\\$ on line 41. I think the new string escape already takes care of this, so I commented it out. I think it might still be necessary for the case of aliases, but I haven't experienced any issues.