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

xonsh default shell doesn't accept sh syntax #31

Open
eugenesvk opened this issue Sep 30, 2021 · 4 comments
Open

xonsh default shell doesn't accept sh syntax #31

eugenesvk opened this issue Sep 30, 2021 · 4 comments

Comments

@eugenesvk
Copy link

If I set xonsh as my default login shell in WSL2, then I can't use wslbridge2 anymore due to incompatible syntax (the bridge only prepares the sh-syntax command), instead I get the following error:

Backend CommandLine: "C:\WINDOWS\System32\wsl.exe" /bin/sh -c 'exec "$(wslpath -u '\''C:\path\to\wslbridge2-backend'\'')" --login --show --cols 120 --rows 30 -060266 -149859 -359317 -- /home/linuxbrew/.linuxbrew/bin/xonsh'
Traceback

X=/home/linuxbrew/.linuxbrew/Cellar/xonsh/0.10.1/libexec/lib/python3.9/site-packages (shortened for readability)

Traceback (most recent call last):
    _failback_to_other_shells(args, err)    run_code_with_cache(
  File "X/xonsh/__amalgam__.py", line 3328, in run_code_with_cache
    tree = self.parser.parse(
  File "X/xonsh/parsers/base.py", line 523, in parse
    tree = self.parser.parse(input=s, lexer=self.lexer, debug=debug_level)
  File "X/xonsh/ply/ply/yacc.py", line 335, in parse
    return self.parseopt_notrack(input, lexer, debug, tracking, tokenfunc)
  File "X/xonsh/ply/ply/yacc.py", line 1203, in parseopt_notrack
    tok = call_errorfunc(self.errorfunc, errtoken, self)
  File "X/xonsh/ply/ply/yacc.py", line 194, in call_errorfunc
    r = errorfunc(token)
  File "X/xonsh/parsers/base.py", line 3459, in p_error
    self._parse_error(msg, self.currloc(lineno=p.lineno, column=p.lexpos))
  File "X/xonsh/parsers/base.py", line 650, in _parse_error
    raise_parse_error(msg, loc, self.xonsh_code, self.lines)
  File "X/xonsh/parsers/base.py", line 238, in raise_parse_error
    raise err
SyntaxError: 080d92d8c20c62294b20abf38050b6da:1:0: ('code: /',)
/bin/sh -c 'exec "$(wslpath -u '\''C:\path\to\wslbridge2-backend'\'')" --login --show --cols 120 --rows 30 -060266 -149859 -359317 -- /home/linuxbrew/.linuxbrew/bin/xonsh'
^

The proper command for this should be something like (newline-split for readability)

"C:\WINDOWS\System32\wsl.exe"
/bin/sh -c
@("exec '{bridge}' {args}".format(
  bridge=$(wslpath -u r"C:\path\to\wslbridge2-backend").rstrip(),
  args="--show --cols 120 --rows 30 -060266 -149859 -359317 -- /home/linuxbrew/.linuxbrew/bin/xonsh"
  ) )

I'll submit a PR shortly that introduces the new syntax either via the less convenient command-flag or via the auto-detection of the default shell via grep "^$(id -un)": /etc/passwd
(I bypassed the appendWslArg function as there is no need to escape the quotes, and I don't understand how relevant the other corrections are :))

@Biswa96
Copy link
Owner

Biswa96 commented Sep 30, 2021

xonsh default shell doesn't accept sh syntax

Isn't this a problem of xonsh?

@eugenesvk
Copy link
Author

xonsh default shell doesn't accept sh syntax

Isn't this a problem of xonsh?

of course not, why would you expect it to accept incorrect syntax?

@Biswa96
Copy link
Owner

Biswa96 commented Oct 1, 2021

If you don't mind I would like to investigate on this. I am not against this. I want to just explore things first.

@eugenesvk
Copy link
Author

Sure thing, maybe you'll find out a nifty trick to make xonsh accept the syntax (with a few more '\'' ;) )!

I think it's mainly the $(wslpath -u substitution that breaks it, so maybe if you do the path conversion internally and switch \ to / it'll be fine, at least
"C:\WINDOWS\System32\wsl.exe" /bin/sh -c 'exec "/mnt/c/path/to/wslbridge2-backend" --help'
doesn't throw any syntax errors and shows teh help message

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 a pull request may close this issue.

2 participants