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

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

Merged
merged 1 commit into from
May 31, 2024

Conversation

frantic1048
Copy link
Contributor

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:

  • proto: 0.35.5

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.

`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`](https://elv.sh/ref/language.html#set).

`$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:

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

For more details, refer to the Elvish documentation on [`$paths`](https://elv.sh/ref/builtin.html#$paths).
@milesj
Copy link
Collaborator

milesj commented May 31, 2024

@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.

@milesj milesj merged commit b9bb000 into moonrepo:master May 31, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants