feat: add support for xonsh #2807

jeremyschlatter · 2021-06-19T17:38:24Z

Description

Adds xonsh init script.

Motivation and Context

This is basically #1265 with merge conflicts resolved and a slightly different implementation of job count.

#1265 got stuck on two issues, which I believe are both now resolved:

NameError: name 'get_next_job_number' is not defined
I worked around this by using len(__xonsh__.all_jobs) rather than get_next_job_number. h/t @anki-code for that implementation.
Escape sequences don't work on $PROMPT variable (using prompt-toolkit) xonsh/xonsh#1403
Seems to now be resolved upstream? The upstream issue isn't closed, but some fix PRs have been merged, and I didn't encounter this problem while testing.

Closes #2704
Closes #1265
Closes #272

Screenshots (if appropriate):

How Has This Been Tested?

I have tested using MacOS
I have tested using Linux
I have tested using Windows

Checklist:

I have updated the documentation accordingly.
I have updated the tests accordingly.

I'm using a local build of starship while I'm waiting for starship/starship#2807 to be merged. The debug build is slower-enough that I can sometimes notice the delay.

Merging to pick up lint fixes.

starship does not have built-in support for xonsh yet, but there are multiple open efforts to add it, including a PR I opened recently: starship/starship#2807 Until that PR lands, I'm switching to using my patched version. (I was already using a patched version on my mac, which I built locally. Better to override it in the nix config so I automatically get it on other machines too.)

andytom · 2021-06-28T20:47:01Z

@vladimyr and @anki-code you have both been working on xonsh support would you be able to take a quick look at this and add your thoughts.

anki-code · 2021-06-29T12:14:50Z

@andytom looks good! Thank you for this!

You can also take a short look into xontrib-prompt-starship to compare your and my implementation.

jeremyschlatter · 2021-06-30T00:56:46Z

Thanks @anki-code!

The two implementations are very similar. For reference, here they are side-by-side.

current PR:

def starship_prompt():
    last_cmd = __xonsh__.history[-1] if __xonsh__.history else None
    status = last_cmd.rtn if last_cmd else 0
    jobs = len(__xonsh__.all_jobs)
    duration = round((last_cmd.ts[1] - last_cmd.ts[0]) * 1000) if last_cmd else 0
    return $(::STARSHIP:: prompt --status=@(status) --jobs=@(jobs) --cmd-duration=@(duration))

xontrib-prompt-starship:

def _starship_prompt(cfg=None):
   with __xonsh__.env.swap({'STARSHIP_CONFIG': cfg} if cfg else {}):
       return __xonsh__.subproc_captured_stdout([
           'starship', 'prompt',
           '--status', str(int( __xonsh__.history[-1].rtn)) if len(__xonsh__.history) > 0 else '0',
           '--cmd-duration' , str(int((__xonsh__.history[-1].ts[1] - __xonsh__.history[-1].ts[0])*1000)) if len(__xonsh__.history) > 0 else '0',
           '--jobs', str(len(__xonsh__.all_jobs))
       ])

anki-code · 2021-06-30T07:42:22Z

Also please take a look into these fixes:

https://github.com/anki-code/xontrib-prompt-starship/blob/25cf80179874e3dd678854797d596eafcd06dcf3/xontrib/prompt_starship.py#L5-L6

__xonsh__.env['STARSHIP_SHELL'] = 'sh'  # Fix https://github.com/anki-code/xontrib-prompt-starship/issues/1
__xonsh__.env['STARSHIP_SESSION_KEY'] = __xonsh__.subproc_captured_stdout(['starship','session']).strip()

anki-code · 2021-06-30T07:45:31Z

And I'm using xonsh subprocess call from the library (__xonsh__.subproc_captured_stdout) to make the code pure python. It allows to translatable the code to python bytecode.

jeremyschlatter · 2021-06-30T08:38:37Z

Thanks for the pointers.

I added STARSHIP_SESSION_KEY.

This PR already sets $STARSHIP_SHELL = "xonsh", which both solves the issue behind anki-code/xontrib-prompt-starship#1 and is better than $STARSHIP_SHELL = "sh" in this context.

Re: pure python, I don't understand how starship benefits from having this code be translatable to python bytecode. Is there any immediate benefit to starship? If not, I think it makes sense to leave the init script here in xonsh syntax, since that syntax is cleaner.

anki-code · 2021-07-01T07:31:38Z

Is there any immediate benefit to starship? If not, I think it makes sense to leave the init script here in xonsh syntax, since that syntax is cleaner.

Nope, you can leave the xonsh syntax.

I added STARSHIP_SESSION_KEY.

Good! Thanks!

src/init/starship.xsh

docs/.vuepress/config.js

davidkna · 2021-07-08T16:47:24Z

src/init/starship.xsh

+
+$PROMPT = starship_prompt
+$STARSHIP_SHELL = "xonsh"
+$STARSHIP_SESSION_KEY = "".join(random.choice(string.ascii_letters + string.digits) for _ in range(16))


I'm getting a NameError: name 'random' is not defined here.

Reproduced on my end.

I'm pretty confused by xonsh's behavior here. I've reduced some of the surprising behavior to a minimal example and asked about it on the xonsh gitter: https://gitter.im/xonsh/xonsh?at=60e7d763951c58084ecb7108

As a work around you could use the uuid module from the python std library. I think this would be enough we just need a unique id for each session

srt(uuid.uuid4()) # or uuid.uuid4().hex

Done. Thanks for that suggestion.

davidkna · 2021-07-08T16:50:45Z

src/init/starship.xsh

+def starship_prompt():
+    last_cmd = __xonsh__.history[-1] if __xonsh__.history else None
+    status = last_cmd.rtn if last_cmd else 0
+    jobs = len(__xonsh__.all_jobs)


After running any command the jobs will sometimes display for me, with only the starship init in my .xonshrc.

Fixed.

Getting job accounting right in xonsh apparently requires regularly calling _clear_dead_jobs.

We could accomplish that here by calling get_next_job_number. In fact, #1265 did exactly that.

Unfortunately, due to what I believe is the same issue as above, we're pretty limited here: we can't use any imported functions or even any names we define outside of this function. As a workaround, I've duplicated enough of the relevant logic in _clear_dead_jobs to ignore dead jobs in our count here.

After running any command the jobs will sometimes display for me, with only the starship init in my .xonshrc.

Incidentally, I reproduced this on xonsh 0.9.27 but not on xonsh's latest main branch. But I don't understand why they would be different. 🤷🏻‍♂️

See #2807 (comment)

davidkna · 2021-07-10T09:28:07Z

src/init/starship.xsh

+    # but we can't use that function because of https://gitter.im/xonsh/xonsh?at=60e8832d82dd9050f5e0c96a
+    jobs = sum(1 for job in __xonsh__.all_jobs.values() if job['obj'] and job['obj'].poll() is None)
+    duration = round((last_cmd.ts[1] - last_cmd.ts[0]) * 1000) if last_cmd else 0
+    return $(::STARSHIP:: prompt --status=@(status) --jobs=@(jobs) --cmd-duration=@(duration))


Now that $STARSHIP_SESSION_KEY works I've found another issue:
Log messages (stderr) from starship don't get displayed (invalid config file). Maybe it needs a workaround like this for PowerShell?

seems to work for me when using https://github.com/anki-code/xontrib-prompt-starship latest and

@ xonsh --version xonsh/0.9.27 @ python -V Python 3.9.2 @ starship --version starship 0.55.0 tag:v0.55.0 commit_hash:09b12a52 build_time:2021-06-20 18:31:06 build_env:rustc 1.53.0 (53cb7b09b 2021-06-17)

Good catch, @davidkna.

This was fixed upstream in xonsh just three weeks ago: xonsh/xonsh#4336

But since no released versions of xonsh contain that fix, we should probably work around it here. I've added a workaround: 1e0bc8d

davidkna

LGTM

feat: add support for xonsh

f19b1b8

jeremyschlatter mentioned this pull request Jun 19, 2021

feat: Add xonsh support #1265

Closed

5 tasks

andytom requested a review from a team June 22, 2021 06:49

Merge branch 'master' into xonsh

d673c43

Merging to pick up lint fixes.

xonsh: add STARSHIP_SESSION_KEY

edae13b

davidkna reviewed Jul 3, 2021

View reviewed changes

src/init/starship.xsh Outdated Show resolved Hide resolved

jeremyschlatter added 3 commits July 3, 2021 11:58

xonsh: implement STARSHIP_SESSION_KEY in xonsh

b7984ca

Merge branch 'master' into xonsh

1c21b3b

Merge branch 'master' into xonsh

425949c

davidkna requested changes Jul 8, 2021

View reviewed changes

jeremyschlatter added 3 commits July 8, 2021 18:21

docs: mention tcsh, elvish, and nu in more places

ab94a7c

xonsh: change STARSHIP_SESSION_KEY implementation

e9e406f

See #2807 (comment)

xonsh: fix jobs implementation

57ea974

jeremyschlatter requested a review from davidkna July 10, 2021 02:20

jeremyschlatter mentioned this pull request Jul 10, 2021

surprising scoping behavior in execx and xonsh -c xonsh/xonsh#4363

Closed

davidkna reviewed Jul 10, 2021

View reviewed changes

xonsh: do not silently discard stderr from starship

1e0bc8d

jeremyschlatter requested a review from davidkna July 15, 2021 09:53

davidkna approved these changes Jul 16, 2021

View reviewed changes

matchai merged commit b1dcd5a into starship:master Jul 16, 2021

jeremyschlatter deleted the xonsh branch July 17, 2021 00:52

NNBnh mentioned this pull request Aug 27, 2021

#TODO info-mono/diyship#1

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for xonsh #2807

feat: add support for xonsh #2807

jeremyschlatter commented Jun 19, 2021 •

edited

Loading

andytom commented Jun 28, 2021

anki-code commented Jun 29, 2021 •

edited

Loading

jeremyschlatter commented Jun 30, 2021

anki-code commented Jun 30, 2021

anki-code commented Jun 30, 2021

jeremyschlatter commented Jun 30, 2021

anki-code commented Jul 1, 2021 •

edited

Loading

davidkna Jul 8, 2021

jeremyschlatter Jul 9, 2021

andytom Jul 9, 2021

jeremyschlatter Jul 9, 2021

davidkna Jul 8, 2021

jeremyschlatter Jul 9, 2021

davidkna Jul 10, 2021

meramsey Jul 14, 2021

jeremyschlatter Jul 15, 2021

davidkna left a comment

feat: add support for xonsh #2807

feat: add support for xonsh #2807

Conversation

jeremyschlatter commented Jun 19, 2021 • edited Loading

Description

Motivation and Context

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

andytom commented Jun 28, 2021

anki-code commented Jun 29, 2021 • edited Loading

jeremyschlatter commented Jun 30, 2021

anki-code commented Jun 30, 2021

anki-code commented Jun 30, 2021

jeremyschlatter commented Jun 30, 2021

anki-code commented Jul 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidkna left a comment

Choose a reason for hiding this comment

jeremyschlatter commented Jun 19, 2021 •

edited

Loading

anki-code commented Jun 29, 2021 •

edited

Loading

anki-code commented Jul 1, 2021 •

edited

Loading