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

Support launching with a specific command #3

Open
lread opened this issue Oct 26, 2021 · 15 comments · May be fixed by #38
Open

Support launching with a specific command #3

lread opened this issue Oct 26, 2021 · 15 comments · May be fixed by #38

Comments

@lread
Copy link
Contributor

lread commented Oct 26, 2021

Currently
membrane.term launches a /bin/bash shell.

This results in the following warning text on macOS 10.15.7:

The default interactive shell is now zsh.
To update your account to use zsh, please run `chsh -s /bin/zsh`.
For more details, please visit https://support.apple.com/kb/HT208050.

Seems that macOS would really rather we use zsh! 🙂

Ideas
I'm not sure the best approach here but offer up:

  • allow user to specify which shell to invoke via a :shell option
  • else use value specified by SHELL environment variable (does that make sense?)
  • else use /bin/bash

Next Steps
Happy to follow up with PR if this all makes sense.

@phronmophobic
Copy link
Owner

Sounds great. Would love a PR!

  1. allow user to specify which shell to invoke via a :shell option
  2. else use value specified by SHELL environment variable (does that make sense?)
  3. else use /bin/bash

Seems like a good plan. Shells are definitely not my area expertise. It seems like $SHELL isn't a perfect default according to stackoverflow (https://stackoverflow.com/a/16375583), but the other options seems like they might actually be more brittle across future OS versions. I like your plan.

@phronmophobic
Copy link
Owner

I just had a thought that it might be interesting to boot directly into emacs and afaict, it works!

bc5072f

One idea I've been kicking around using membrane.term as a way to provide a usable code-editor for membrane. It might require a little configuration to hide the frame, but it seems plausible.

Embedding a code editor made me wonder if you could embed a number of other cli tools.
In place of ["/bin/bash" "-l"], I also tried ["/usr/bin/env" "vim"], ["git" "log"], ["clj"], and ["htop"] which all seemed to work. Still not sure what this implies or if I'm misusing pty, but I think there's some really cool use cases that might be hidden here, but not quite sure what.

@lread
Copy link
Contributor Author

lread commented Oct 28, 2021

Interesting! So maybe instead of adding a :shell option we add a :cmd option.

@phronmophobic
Copy link
Owner

Yea, that makes sense. It could potentially be useful for the use case you mention of taking screenshots for cli tools. You can bypass the shell and skip to the good part.

@lread
Copy link
Contributor Author

lread commented Oct 28, 2021

Neat!

How do you feel about bringing in deps?

I'm thinking of babashka.process's nice tokenization support.

@phronmophobic
Copy link
Owner

I'm pretty open to adding dependencies that solve problems. Obviously, if it's incredibly huge, it's worth thinking about, but that's very rare for clojure libs.

@lread
Copy link
Contributor Author

lread commented Oct 30, 2021

So while I work on #5, I'm thinking ahead about the new cmd idea.

We'll have (assume the clojure -M... prefix, replace --cmd for :cmd for -X usage):

  • run-term (cmd defaults as proposed for shell option - with -l)
  • run-term --cmd "/bin/zsh -l"
  • run-term --cmd "git log"

But what about run-script?
Currently run-script :path effectively types or plays or enters (or what's a better word? maybe applies?) the script to the terminal.

What do you think of:

  • run-script --play script.sh (rename path to play, cmd defaults as for run-term)
  • run-script --cmd "clj" --play script.clj"

Watha think? With the cmd idea, is the play concept still interesting/valuable?

@phronmophobic
Copy link
Owner

phronmophobic commented Oct 30, 2021

We'll have (assume the clojure -M... prefix, replace --cmd for :cmd for -X usage):

run-term (cmd defaults as proposed for shell option - with -l)
run-term --cmd "/bin/zsh -l"
run-term --cmd "git log"

I assume you're using babashka.process to tokenize "git log" into ["git" "log"]. Are there any common shells where the tokenization might differ? That's probably fine, but worth checking.

What do you think of:

run-script --play script.sh (rename path to play, cmd defaults as for run-term)
run-script --cmd "clj" --play script.clj"

Play does sound better than "path". It's not entirely clear that run-script and run-term should be different commands. It would also be possible to add options like --headless and --screenshot, but maybe it's less confusing if they're separate commands.

Watha think? With the cmd idea, is the play concept still interesting/valuable?

I think so. One of the original intended use cases was to be able to automate taking screenshots for terminal utilities. I'm not sure how to produce the following image without a --play option.

terminal

The script was:

export PS1="$ "
cd
cd workspace/deep-diff2
clear
clojure
(require '[lambdaisland.deep-diff2 :as ddiff])
(ddiff/pretty-print (ddiff/diff {:a 1 :b 2} {:a 1 :c 3}))

Additionally, You could theoretically also use it for testing how a command line program responds to ctrl+c, ctrl+z, and other input sequences.

@lread
Copy link
Contributor Author

lread commented Oct 30, 2021

Thanks @phronmophobic!

I assume you're using babashka.process to tokenize "git log" into ["git" "log"].

Yeah, that's the thought.

Are there any common shells where the tokenization might differ? That's probably fine, but worth checking.

The convenience might turn out to be inconvenient/problematic in some cases.
I shall ponder more on this.

It's not entirely clear that run-script and run-term should be different commands. It would also be possible to add options like --headless and --screenshot, but maybe it's less confusing if they're separate commands.

Hmmm, interesting point.
I think maybe keeping them separate might be more understandable as they have different goals - and there are options specific to run-script.
But run-script might be renamed to screenshot because that's its ultimate outcome.

And on keeping a play option: nice example, sounds good, thanks!

@phronmophobic
Copy link
Owner

The convenience might turn out to be inconvenient/problematic in some cases.
I shall ponder more on this.

For the cli, I think it's ok if to not support every use case if it makes 80-90% of use cases easier. If it's uncommon for a shell to not have similar tokeniztion, it's always possible to use the programmatic API as a workaround.

I think maybe keeping them separate might be more understandable as they have different goals - and there are options specific to run-script.
But run-script might be renamed to screenshot because that's its ultimate outcome.

👍

And on keeping a play option: nice example, sounds good, thanks!

It would also leave open the possibility for a "record" feature in the future.

@lread
Copy link
Contributor Author

lread commented Oct 30, 2021

Cool, but my most important question is: what's the difference between phronmophobic (your username) and phronemophobic (seen in your project namespaces) and are these related to phronesis and does this mean you have a fear of practical wisdom?

@phronmophobic
Copy link
Owner

It's been my username since middleschool, but the original definition I read was that phronemophobia is the "fear of thinking". I definitely have a tendency to over think things :D. The reason my username is spelled wrong was because the first platform that I used had a 14 character username limit. I own phronemophobic.com which is why it's used in all of the project namespaces. 😄

@lread
Copy link
Contributor Author

lread commented Oct 30, 2021

Thanks! The fact that you've put a lot of thought into your username makes total sense! 🙂

@lread
Copy link
Contributor Author

lread commented Dec 4, 2021

@phronmophobic, gonna look at this one.

Current thoughts:

  1. The screenshot --play/:play option can become optional (currently mandatory)
  2. New --command/:command option
    1. If unspecified, defaulting to a reasonable shell still makes sense:
      1. on macOS and Linux will follow the original proposal
      2. on Windows will default to Powershell (not saying this means we support Windows correctly yet, but we would get past launch)
    2. For programmatic API, :command ["prog" "arg1" "arg2"]
    3. For command line API, --command "prog arg1 arg2", the value of which we will translate to programmatic API form via babashka.process/tokenize.

Sound good to you too?

@lread lread changed the title Consider which shell to launch Support launching with a specific command Dec 4, 2021
lread added a commit to lread/membrane.term that referenced this issue Dec 4, 2021
Added option :command to API and --command command line.

- screenshot :play/--play option is now optional
- Adapted sample play scripts to run /bin/bash as part
  of setup. On my system I run zsh with powerlevel10k and
  my fancy prompts were getting into README screenshots.
- On macOS and Linux :command defaults to $SHELL else
  /bin/bash
- Included support to default :command to powershell if on
  Windows. This gets us past launch, but there
  are still be many issues on Windows.
  Contributes to phronmophobic#29
- See README and docstrings for details on usage

Closes phronmophobic#3
lread added a commit to lread/membrane.term that referenced this issue Dec 4, 2021
Added option :command to API and --command command line.

- screenshot :play/--play option is now optional
- Adapted sample play scripts to run /bin/bash as part
  of setup. On my system I run zsh with powerlevel10k and
  my fancy prompts were getting into README screenshots.
- On macOS and Linux :command defaults to $SHELL else
  /bin/bash
- Included support to default :command to powershell if on
  Windows. This gets us past launch, but there
  are still be many issues on Windows.
  Contributes to phronmophobic#29
- See README and docstrings for details on usage

Closes phronmophobic#3
lread added a commit to lread/membrane.term that referenced this issue Dec 4, 2021
Added option :command to API and --command command line.

- screenshot :play/--play option is now optional
- Adapted sample play scripts to run /bin/bash as part
  of setup. On my system I run zsh with powerlevel10k and
  my fancy prompts were getting into README screenshots.
- On macOS and Linux :command defaults to $SHELL else
  /bin/bash
- Included support to default :command to powershell if on
  Windows. This gets us past launch, but there
  are still be many issues on Windows.
  Contributes to phronmophobic#29
- See README and docstrings for details on usage

Closes phronmophobic#3
lread added a commit to lread/membrane.term that referenced this issue Dec 4, 2021
Added option :command to API and --command command line.

- screenshot :play/--play option is now optional
- Adapted sample play scripts to run /bin/bash as part
  of setup. On my system I run zsh with powerlevel10k and
  my fancy prompts were getting into README screenshots.
- On macOS and Linux :command defaults to $SHELL else
  /bin/bash
- Included support to default :command to powershell if on
  Windows. This gets us past launch, but there
  are still be many issues on Windows.
  Contributes to phronmophobic#29
- See README and docstrings for details on usage

Closes phronmophobic#3
lread added a commit to lread/membrane.term that referenced this issue Dec 5, 2021
Added option :command to API and --command command line.

- screenshot :play/--play option is now optional
- Adapted sample play scripts to run /bin/bash as part
  of setup. On my system I run zsh with powerlevel10k and
  my fancy prompts were getting into README screenshots.
- On macOS and Linux :command defaults to $SHELL else
  /bin/bash
- Included support to default :command to powershell if on
  Windows. This gets us past launch, but there
  are still be many issues on Windows.
  Contributes to phronmophobic#29
- See README and docstrings for details on usage

Closes phronmophobic#3
lread added a commit to lread/membrane.term that referenced this issue Dec 5, 2021
Added option :command to API and --command command line.

- screenshot :play/--play option is now optional
- Adapted sample play scripts to run /bin/bash as part
  of setup. On my system I run zsh with powerlevel10k and
  my fancy prompts were getting into README screenshots.
- On macOS and Linux :command defaults to $SHELL else
  /bin/bash
- Included support to default :command to powershell if on
  Windows. This gets us past launch, but there
  are still be many issues on Windows.
  Contributes to phronmophobic#29
- See README and docstrings for details on usage

Closes phronmophobic#3
@lread lread linked a pull request Dec 5, 2021 that will close this issue
@phronmophobic
Copy link
Owner

Sound good to you too?

Sounds like a good plan!

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