Skip to content

Fix install.sh output#842

Merged
jdx merged 1 commit intojdx:mainfrom
xanderificnl:main
Aug 25, 2023
Merged

Fix install.sh output#842
jdx merged 1 commit intojdx:mainfrom
xanderificnl:main

Conversation

@xanderificnl
Copy link
Copy Markdown
Contributor

The behavior of the echo command and its handling of escape sequences, such as \n, can exhibit variability across different implementations of shells and utilities. This variability was evident during my experience while executing the install.sh script on a newly installed Fedora system:

[xander@fedora ~]$ curl https://rtx.pub/install.sh | sh
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4302  100  4302    0     0  78350      0 --:--:-- --:--:-- --:--:-- 79666
rtx: installing rtx...
rtx: installed successfully to /home/xander/.local/share/rtx/bin/rtx
rtx: run the following to activate rtx in your shell:
echo "eval \"\$(/home/xander/.local/share/rtx/bin/rtx activate bash)\"" >> ~/.bashrc\n # <--newline
rtx: this must be run in order to use rtx in the terminal
rtx: run `rtx doctor` to verify this is setup correctly

Furthermore, a direct copy-paste of the command could potentially lead to unintended consequences. For instance, when executed in a bash shell, the command may inadvertently remove the trailing backslash:

[xander@fedora ~]$ echo "eval \"\$(/home/xander/.local/share/rtx/bin/rtx activate bash)\"" >> ~/.bashrc\n
[xander@fedora ~]$ cat .bashrcn # note the `n` at the end
eval "$(/home/xander/.local/share/rtx/bin/rtx activate bash)"
[xander@fedora ~]

Podman testing showed consistent echo behavior with bash across distributions, except for Debian and Ubuntu (using dash) which interpret escape sequences by default. Thus, replacing the newlines with empty info calls appears to be the most portable solution.

The behavior of the `echo` command and its handling of escape sequences, such as `\n`, can exhibit variability across different implementations of shells and utilities. This variability was evident during my experience while executing the `install.sh` script on a newly installed Fedora system:

```bash
[xander@fedora ~]$ curl https://rtx.pub/install.sh | sh
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4302  100  4302    0     0  78350      0 --:--:-- --:--:-- --:--:-- 79666
rtx: installing rtx...
rtx: installed successfully to /home/xander/.local/share/rtx/bin/rtx
rtx: run the following to activate rtx in your shell:
echo "eval \"\$(/home/xander/.local/share/rtx/bin/rtx activate bash)\"" >> ~/.bashrc\n # <--newline
rtx: this must be run in order to use rtx in the terminal
rtx: run `rtx doctor` to verify this is setup correctly
```

Furthermore, a direct copy-paste of the command could potentially lead to unintended consequences. For instance, when executed in a bash shell, the command may inadvertently remove the trailing backslash:

```bash
[xander@fedora ~]$ echo "eval \"\$(/home/xander/.local/share/rtx/bin/rtx activate bash)\"" >> ~/.bashrc\n
[xander@fedora ~]$ cat .bashrcn # note the `n` at the end
eval "$(/home/xander/.local/share/rtx/bin/rtx activate bash)"
[xander@fedora ~]
```

Podman testing showed consistent `echo` behavior with bash across distributions, except for Debian and Ubuntu (using dash) which interpret escape sequences by default. Thus, replacing the newlines with empty `info` calls appears to be the most portable solution.
@jdx jdx enabled auto-merge (squash) August 25, 2023 00:07
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 25, 2023

Codecov Report

Patch coverage has no change and project coverage change: +6.33% 🎉

Comparison is base (892c752) 81.27% compared to head (e450166) 87.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #842      +/-   ##
==========================================
+ Coverage   81.27%   87.60%   +6.33%     
==========================================
  Files         132      132              
  Lines       11533    11533              
==========================================
+ Hits         9373    10104     +731     
+ Misses       2160     1429     -731     

see 29 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jdx jdx merged commit e2baa73 into jdx:main Aug 25, 2023
jdx pushed a commit that referenced this pull request Apr 9, 2024
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 this pull request may close these issues.

2 participants