phronmophobic / membrane.term

A terminal emulator in pure clojure
Eclipse Public License 1.0
53 stars 1 forks source link

Terminal can be launched with a specified command #38

Open lread opened 2 years ago

lread commented 2 years ago

Added option :command to API and --command command line.

Closes #3

lread commented 2 years ago

@phronmophobic, previously the hardcoded launch command was ["/bin/bash" "-l"]. The new default loses the -l arg, do we need to retain it?

phronmophobic commented 2 years ago

@phronmophobic, previously the hardcoded launch command was ["/bin/bash" "-l"]. The new default loses the -l arg, do we need to retain it?

To be honest, I'm not sure. From the docs, it seems like -l makes sense, but I don't remember where that came from. Additionally, I'm not 100% sure we should be automatically copying environment variables either.

It would be interesting to check what other terminal emulators are doing. Additionally, I've only been running run-term from the command line or from a repl that was launched from the command line. I wonder if it makes a difference to launch run-term more directly, eg ProcessBuilder with java -cp <classpath> <uber.jar>.

phronmophobic commented 2 years ago

Here's some interesting info in kitty: https://github.com/kovidgoyal/kitty/blob/c8c6f8691f4aa84f4d798428af268319b898798f/kitty/child.py#L257

phronmophobic commented 2 years ago

More references: https://github.com/alacritty/alacritty/issues/645 https://github.com/kovidgoyal/kitty/issues/247

As far as I can tell, it seems like passing --login is the way to go.

lread commented 2 years ago

Thanks for the references! Only skimmed, will read more carefully tomorrow. They seem to be macOS related? Does Linux also need the --login arg?

I'm thinking that turfing automatically using $SHELL might make sense, just because we might not be able to guess what the correct shell args are.

phronmophobic commented 2 years ago

They seem to be macOS related? Does Linux also need the --login arg?

I've read these tickets a few times now and I'm still not 100% sure. It seems like on macOS, you want to login mainly to load .bash_profile. It doesn't seem like this is necessary on linux and might actually subvert common expectations.

Based on the tickets, it looks like rather than passing -l arg, the command is prefixed with - on macOS:

I'm thinking that turfing automatically using $SHELL might make sense, just because we might not be able to guess what the correct shell args are.

I think the above fix also would work if we use $SHELL as fallback default. This is what iTerm2 does:

Even if the above change would fix using $SHELL, that doesn't mean we should. Since we do offer a command line interface, I think it's reasonable to use the $SHELL environment variable as a fallback default. Thoughts?

lread commented 2 years ago

Thanks for all the follow-up investigations! I've been distracted with advent-of-code puzzles!

From the bash man page (same text for macOS and Linux), I see:

A login shell is one whose first character of argument zero is a -, or one started with the --login option.

So maybe the 2 techniques are equivalent?

But what if I am using a different shell? I'm not very shell-variant savvy but have used zsh.

The man page for zshoptions includes:

        LOGIN (-l, ksh: -l)
              This is a login shell.  If this option is not explicitly set, the shell becomes a login shell if the first character of the argv[0] passed to the shell is a `-'.

As the veil of naivete lifts, I expect this is probably a convention?

So yeah, maybe we can still default to $SHELL and we'll document that we launch as a login shell using bash - convention. If this does not work for someone's $SHELL, they can override the default command.

But the question remains, only for macOS or also for Linux? I haven't fully digested this yet, but it seems maybe (?) this is only needed for macOS.

phronmophobic commented 2 years ago

So maybe the 2 techniques are equivalent?

That's what I was thinking as well. The reason I think we should prefer using the "-" prefix over the --login arg is simply because that's what other terminal emulators are doing which makes it more likely that our default will match user expectations.

But the question remains, only for macOS or also for Linux? I haven't fully digested this yet, but it seems maybe (?) this is only needed for macOS.

As far as I can tell, requiring a login shell is only required for macOS. That's also how the other terminal emulators seem to behave.

lread commented 2 years ago

Okay, sounds good, thanks! I shall follow up with a commit that reflects our current understanding. (update: I did not really understand! See next comment for recap)

lread commented 2 years ago

Okay, better recap the macOS login shell issue.

Due to the way Apple decided to initialize shells, on macOS only, new shells should be created as login terminals (our surveyed terminals all do this). This is not needed/wanted/necessary on Linux.

There are three options for shells macOS:

  1. do nothing. Do nothing - don't launch as login terminal
  2. args. Use -l or --login shell option to indicate login shell.
  3. prefix hack. Launch the shell in such a way that it is described with a - prefix. For example, a launch of bash would be described as -bash.

Here are the possible valid values for our new :command option

  1. user command. user-specified command (non-shell)
  2. user shell. user-specified shell
  3. default shell. either $SHELL value else /bin/bash

Note that membrane.term does not distinguish between user command and user shell, it just sees a :command.

Okay, now let's look at our options:

  1. [REJECTED] do nothing
    1. user command - this would be fine
    2. user shell - shell may not operate as expected
    3. default shell - shell may not operate as expected
  2. [CANDIDATE] args
    1. user command
      1. [NOGO] we would not want to tack on shell args to a user command
      2. the user would be in control of all args for command
    2. user shell
      1. [NOGO] we could tack on args to a user-specified shell, but would then would have to detect that the user-specified a shell vs some other command.
      2. or more likely, the user would tack them on
    3. default shell
      1. we could easily tack on shell args to defaults
      2. [POTENTIAL ISSUE] are we confident that these are the right args for all $SHELLs? maybe not, but if not the user could explicitly override the default.
  3. [CURRENT CHOICE] prefix hack - [ADDS WEIGHT] the dash prefix seems to be the preferred technique in terminals we've surveyed
    1. user command - we expect this should be harmless, but if we learn otherwise, we can later adapt with a boolean :dash-prefix-command? option.
    2. user shell - hack can be silently added to any command, it could be redundant if the user also specifies shell -l or --login shell args, but we think, not problematic.
    3. default shell - hack can be silently added to any command

Implementation complexities. Option 3 is more technically complex for membrane.term than option 2. We use pty4j to launch our :command. It's generic API does not allow for overriding the description of launched executable. This probably makes sense because this is likely(?) only technically possible on macOS and Linux platforms. Pty4j does, though, have a lower level API which we will explore for macOS.

lread commented 2 years ago

Had a quick peek just now.

I so almost had a clever solution to dash prefixing on macOS with pty4j.

I figured I could create a Clojure proxy over com.pty4j.unix.UnixPtyProcess and override the exec method wherein I'd do my dash prefix bidding.

Alas, the exec method is not public, it is package-private. I'll take another look at this sometime soon.

lread commented 2 years ago

Even though we will likely do something here in the meantime, it would be better if pty4j supported this in its higher-level APIs. I've raised an issue over there to see if there is any interest.