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

add xonsh support #84

Merged
merged 5 commits into from
Dec 8, 2021
Merged

add xonsh support #84

merged 5 commits into from
Dec 8, 2021

Conversation

hall
Copy link
Contributor

@hall hall commented Sep 7, 2021

Mostly straightforward changes; not a xonsh or rust expert but it works on my machine 😄 a few notes to help explain why I did things the way I did:

  • xonsh doesn't like the \e color escapes (but works fine with \033)
  • xonsh runs in a python subprocess
  • xonsh doesn't let users embed arbitrary commands into the prompt without first adding them to the PROMPT_FIELDS dict.

@sbstp
Copy link
Owner

sbstp commented Sep 8, 2021

Nice, thanks. In the other shells I've added hooks to constantly reset KUBECONFIG to the right value (the one in KUBIE_KUBECONFIG). This is done to avoid errors if someone or a tool does an export KUBECONFIG=... in a kubie shell. Is it something that can be implemented with xonsh?

@hall
Copy link
Contributor Author

hall commented Sep 8, 2021

Ah, good catch! Yep, I've added that functionality now.

However, I'm only now realizing that the prompt doesn't actually get updated after initialization. I'll convert this to a draft until I figure that out.

@hall hall marked this pull request as draft September 8, 2021 13:34
@hall hall marked this pull request as ready for review September 8, 2021 14:19
@sbstp
Copy link
Owner

sbstp commented Sep 10, 2021

I tried your branch with xonsh, I got this error just after picking the context. I'm using Debian bullseye so the xonsh version is a tad behind, but not that much.

simon@debian ~/projects/kubie hall-xonsh $ $XONSH_SHOW_TRACEBACK = True
simon@debian ~/projects/kubie hall-xonsh $ kubie ctx
xonsh: To log full traceback to a file set: $XONSH_TRACEBACK_LOGFILE = <filename>
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/xonsh/__amalgam__.py", line 17922, in xonsh_script_run_control
    run_script_with_cache(filename, execer, ctx)
  File "/usr/lib/python3/dist-packages/xonsh/__amalgam__.py", line 3214, in run_script_with_cache
    ccode = compile_code(filename, code, execer, glb, loc, mode)
  File "/usr/lib/python3/dist-packages/xonsh/__amalgam__.py", line 3173, in compile_code
    ccode = execer.compile(code, glbs=glb, locs=loc, mode=mode, filename=filename)
  File "/usr/lib/python3/dist-packages/xonsh/__amalgam__.py", line 21438, in compile
    tree = self.parse(input, ctx, mode=mode, filename=filename, transform=transform)
  File "/usr/lib/python3/dist-packages/xonsh/__amalgam__.py", line 21398, in parse
    tree, input = self._parse_ctx_free(input, mode=mode, filename=filename)
  File "/usr/lib/python3/dist-packages/xonsh/__amalgam__.py", line 21522, in _parse_ctx_free
    tree = self.parser.parse(
  File "/usr/lib/python3/dist-packages/xonsh/parsers/base.py", line 497, in parse
    tree = self.parser.parse(input=s, lexer=self.lexer, debug=debug_level)
  File "/usr/lib/python3/dist-packages/ply/yacc.py", line 333, in parse
    return self.parseopt_notrack(input, lexer, debug, tracking, tokenfunc)
  File "/usr/lib/python3/dist-packages/ply/yacc.py", line 1120, in parseopt_notrack
    p.callable(pslice)
  File "/usr/lib/python3/dist-packages/xonsh/parsers/base.py", line 2253, in p_atom_expr
    p[0] = self.apply_trailers(p[1], p[2])
  File "/usr/lib/python3/dist-packages/xonsh/parsers/base.py", line 2247, in apply_trailers
    assert False
AssertionError
error running xonsh run control file '/tmp/kubie-xonshrcVUMSyd.xonsh': 
simon@debian ~/projects/kubie hall-xonsh $ xonsh --version
xonsh/0.9.25

@@ -15,6 +16,7 @@ impl ShellKind {
Some(match name {
"bash" | "dash" => ShellKind::Bash,
"fish" => ShellKind::Fish,
"xonsh" | "python" => ShellKind::Xonsh,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's hope not too many shells are made with Python 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured we'd let them cross that bridge when they get here 😁 The alternative would probably be to, if python was detected, strip the interpreter and then reevaluate the remaining string as if it were the original command.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we'll cross the bridge when we get to it

@sbstp
Copy link
Owner

sbstp commented Sep 10, 2021

Code looks good to me. It's just a matter of ironing out a few bugs I think.

@hall
Copy link
Contributor Author

hall commented Sep 10, 2021

Interesting. You're only one patch version behind me (on nixos 21.05). I just tried the appimage from xonsh's repo (the 0.9.25 release --version flag actually returned xonsh/0.9.24.dev223 but 🤷 close enough) and still couldn't reproduce your error. Probably something different about our environments. I'll keep testing.

@sbstp
Copy link
Owner

sbstp commented Sep 10, 2021

I tried installing xonsh 0.10.1 with pipx, pipx install xonsh[full] but I don't see a custom prompt like in bash or zsh. Is it supposed to not have a prompt?

@hall
Copy link
Contributor Author

hall commented Sep 10, 2021

It should update (prepend, by default) the prompt, yeah.

print($PROMPT)

Should have something like [{ctx}|{ns}] at the beginning.

@sbstp
Copy link
Owner

sbstp commented Sep 10, 2021

simon@debian ~/projects $ print($PROMPT)
{env_name}{BOLD_GREEN}{user}@{hostname}{BOLD_BLUE} {cwd}{branch_color}{curr_branch: {}}{RESET} {BOLD_BLUE}{prompt_end}{RESET}

I start in a bash shell first, then do xonsh to start a xonsh shell. Then I do kubie ctx and pick a context. I wonder if bash could be affecting things.

@hall
Copy link
Contributor Author

hall commented Sep 10, 2021

Hm, I did xonsh -> bash -> xonsh --no-rc and the prompt was there. It should only not add the prompt if it's disabled in kubie's settings: https://github.com/sbstp/kubie/pull/84/files#diff-c27cd8b25abdaea337f9e4966c67aac8e4bb06a79f3fa7cbcd16840383ef18ddR39

It does it at the 3rd to last line in the generated file:

[k|default][23:07:53]  bryton@thinkpad ~ 
> cat /tmp/kubie-xonshrcegwXFe.xonsh | tail -n 3
    $PROMPT = $KUBIE_PROMPT + $PROMPT

del $KUBIE_PROMPT

@hall
Copy link
Contributor Author

hall commented Sep 10, 2021

Alright -- I think I've fixed the AssertionError. I was able to reproduce with this Dockerfile:

FROM debian:bullseye

RUN apt update && \
    apt install -y \
        cargo \
        procps \
        vim \
        xonsh

WORKDIR /app
COPY Cargo.toml Cargo.lock ./
COPY src src
RUN cargo install --path .

ENV PATH /root/.cargo/bin:$PATH

ENTRYPOINT xonsh

then

> docker build -t kubie-debug .
. . .
> docker run -it --rm  -v $HOME/.kube:/root/.kube kubie-debug

and, once inside the container

kubie ctx <context>

For some reason, it didn't like the array index notation so I just replaced that with a call to pop() 🤷 let me know if that fixes it on your end.

@hall
Copy link
Contributor Author

hall commented Dec 7, 2021

These changes have been working pretty well on my end; let me know if there's something else that needs fixed/updated before it can be merged.

@sbstp
Copy link
Owner

sbstp commented Dec 8, 2021

Yeah, sorry. I was under the assumption that the bug I found was still there.

@sbstp
Copy link
Owner

sbstp commented Dec 8, 2021

I re-tested it and it looks good so I will merge it.

@sbstp sbstp merged commit 528c192 into sbstp:master Dec 8, 2021
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.

2 participants