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

Make install.sh more robust to not being root #258

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

plutonium-239
Copy link
Contributor

Instead of just failing if sudo is not granted, the install script now tries to:

  1. move ./spf to ~/.local/bin (which is the correct path for user executables according to the XDG Base Directory Spec)
  2. Make that directory if it doesn't exist
  3. Checks if this dir is in the path, and if not export path to include it (and add to bashrc)
  4. improves error messages accordingly.

Here is an example of how it looks with these changes: (ignore the actual colors, my terminal's color profile is different)
image

One question I had is should I also mention that ~/.local/bin has been added to ~/.bashrc (so that users of other shells i.e. fish/zsh/xonsh etc) would be able to add it to their configs themselves? I am in favour of adding it.
Alternatively, we can try detecting shell and run respective command but not even fzf takes that approach - it asks you to run the right file yourself (out of some provided scripts)

Instead of just failing if `sudo` is not granted, the install script now tries to move `./spf` to `~/.local/bin` (which is the correct path for user executables according to the [XDG Base Directory Spec](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)) and improves error messages accordingly.
Copy link

netlify bot commented Jun 18, 2024

Deploy Preview for superfile ready!

Name Link
🔨 Latest commit f3d209a
🔍 Latest deploy log https://app.netlify.com/sites/superfile/deploys/6671c465a702ae0009f645e5
😎 Deploy Preview https://deploy-preview-258--superfile.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@yorukot yorukot merged commit 1dabeec into yorukot:main Jun 19, 2024
4 checks passed
@yorukot
Copy link
Owner

yorukot commented Jun 19, 2024

thank you for your contribution, this look good to me!

@plutonium-239
Copy link
Contributor Author

Thanks!
What do you think about the question I wrote about users of shells other than bash?

@yorukot
Copy link
Owner

yorukot commented Jun 19, 2024

I apologize for missing that part of your message. :(

Regarding your question about users of shells other than bash, I believe automating the detection of the user's shell is a great idea. If there's a straightforward way to achieve this in a shell script without requiring additional user input, that would be ideal. The goal of the install script should be to minimize the need for users to manually configure settings or enter additional information.

@plutonium-239
Copy link
Contributor Author

No worries! :)

I agree, it should be automated. We could use a case statement on the env variable SHELL, which works reliably. I'll push a change with this.

yorukot added a commit that referenced this pull request Jun 20, 2024
Continued #258: `install.sh` Add support for multiple shells
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