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

Install scripts bash improvements #266

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

opatry
Copy link

@opatry opatry commented Nov 4, 2024

Original motivation was to let me call the util/install_macos.sh script from anywhere, which was my original intent to install the fonts.
I downloaded and extracted the zip somewhere, then from the working dir I was in, I just called ~/Downloads/monaspace-v1.101/util/install_macos.sh and it failed.

With the first commit, it can now be called from anywhere.

While I was as it, I suggested some improvements (maybe a bit opinionated?), feel free to request removal of some if it doesn't suit your practices.
There are several commits to better isolate changes.
Commits detailed message contain motivation/explanation when relevant if you need.

The original scripts were forcing the caller to be in the monospace directory and calling `./util/install_XXX.sh`.
You can now execute `install_XXX.sh` from wherever you want.
ex: `~/Downloads/monaspace-v1.101/util/install_macos.sh`
`#!/usr/bin/env` searches `PATH` for `bash`, and `bash` is not always in `/bin`, particularly on non-Linux systems.
See https://en.wikipedia.org/wiki/Shebang_(Unix)#Portability
Would easily allow letting caller choose output dir for instance.

```bash
fonts_dir="${1:-"${HOME}/Library/Fonts"}"
```

Note:
Used `${HOME}` as a replacement of `~` which doesn't expands within quotes.
See https://www.shellcheck.net/wiki/SC2088
@opatry
Copy link
Author

opatry commented Nov 4, 2024

I just noticed I went a bit quickly and missed PR #218

My version follows the same logic but goes a step beyond and also handles macOS though.

@opatry
Copy link
Author

opatry commented Nov 4, 2024

Also relates to PR #239

@opatry opatry mentioned this pull request Nov 4, 2024
@opatry
Copy link
Author

opatry commented Nov 4, 2024

Also related to PR #66 & issue #63

@JRodez
Copy link

JRodez commented Nov 29, 2024

Just want to say that this is an important PR because it fixes the shebang.

@heathercran heathercran added this to the 1.2 milestone Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants