moonrepo / starbase

Framework for building performant command line applications and developer tools.
MIT License
68 stars 6 forks source link

fix: correct elvish shell syntax for `Elvish.format_path_export()` #74

Closed frantic1048 closed 1 month ago

frantic1048 commented 1 month ago

Current Elvish.format_path_export() would cause syntax error on proto setup --shell elvish(proto setup documentation):

Multiple compilation errors:
  need = and right-hand-side
    /Users/chino/.config/elvish/rc.elv:28:58: set paths [$E:PROTO_HOME/shims $E:PROTO_HOME/bin $@paths]
  lvalue must be valid literal variable names
    /Users/chino/.config/elvish/rc.elv:28:11-57: set paths [$E:PROTO_HOME/shims $E:PROTO_HOME/bin $@paths]

versions:


By the way, the test errors_no_ext in crates/archive/tests/archive_test.rs fails when the terminal is too narrow. It appears that the wrapped and prettily formatted panic message causes this. I am not sure how to fix this. 🤔

image
cargo test
...
failures:

---- errors_no_ext stdout ----
thread 'errors_no_ext' panicked at crates/archive/tests/archive_test.rs:23:30:
called `Result::unwrap()` on an `Err` value: archive::unknown_format

  × Unable to handle archive /var/folders/yg/
  │ h6lqbjy523ngfd9h6lk1_nz80000gn/T/.tmpfqMVPb/out, could not
  │ determine format.

note: panic did not contain expected string
      panic message: `"called `Result::unwrap()` on an `Err` value: \u{1b}[31marchive::unknown_format\u{1b}[0m\n\n  \u{1b}[31m×\u{1b}[0m Unable to handle archive \u{1b}[38;5;38m/var/folders/yg/\n  \u{1b}[31m│\u{1b}[0m h6lqbjy523ngfd9h6lk1_nz80000gn/T/.tmpfqMVPb/out\u{1b}[0m, could not\n  \u{1b}[31m│\u{1b}[0m determine format.\n"`,
 expected substring: `"could not determine"`
...

Explaination

set

set is a special command for assigning values to variables in Elvish. It requires an = to separate the variable name from the value.

The use of = is necessary because it allows for multiple assignments, such as set x y z = 1 2 3, in my opinion.

For more details, refer to the Elvish documentation on set.

paths

$paths is a variable containing a list of strings that are always synchronized with the PATH environment variable. Since it is a variable, we use set to assign values to it. Therefore, the correct way to assign values to $paths is:

set paths = [$E:PROTO_HOME/shims $E:PROTO_HOME/bin $@paths]

For more details, refer to the Elvish documentation on $paths.

set-env

[!NOTE]
This is not related to changes in this PR, but the information may be helpful.

set-env is a built-in command used to set environment variables. It requires an environment variable name and a value as arguments. Therefore, ways to set an environment variable are as follows:

set-env PROTO_HOME {~}/.proto

# the same as
set E:PROTO_HOME = {~}/.proto

# Unlike 'set', 'set-env' supports dynamic variable names
var env_name = PROTO_HOME
set-env $env_name {~}/.proto

For more details, refer to the Elvish documentation on set-env.

milesj commented 1 month ago

@frantic1048 Thanks for this! I wasn't 100% positive the elvish syntax was correct.

As for the test, I guess just ignore it for now.