Repository navigation
Add Support for Multiple Font Names in install.sh - #2086
Conversation
|
@all-contributors add danifbento code |
|
I've put up a pull request to add @danifbento! 🎉 |
Finii
left a comment
There was a problem hiding this comment.
Thanks for the PR!
I add some observations below.
There is no need to change the commit (right now), this is mostly about discussing.
Also it would have been nice to use the PR template. As this is already the 2nd or 3rd PR that looks like LLM generated and they all omitted the template I changed the contribution guidelines a bit 😬
(Also no need to change anything here, just for future PRs)
|
Hello, thanks for your feedback. In fact I only saw the way that descriptions are being done after submitting (looking at other PR, didn't saw the template in the contributors page, maybe my fault I didn't check first). I asked a little help for the description yes 😄 I will change it and improve it. Regarding other comments, during the day I will try to reply to you and fix them. Thanks again for your fast feedback! |
|
If you change your commit, please also increase the script version To check with I often forget that myself on script changes, but when someone reports problems with the script this becomes essential. |
|
Updated the PR description and fixed some of the suggestions already. |
|
I added some more changes :-} |
|
Thanks, the changes seems to be a big improvement 😄 Thanks for that. |
| # ----------- | ||
|
|
||
| VERBOSE="${VERBOSE:-1}" | ||
| VERBOSE="${VERBOSE:-0}" |
There was a problem hiding this comment.
🤔 I can not remember why I did that, but probably it predates the --silent flag, and then we needed to start with 0 (if we can only ADD verbosity).
Yes. Will change the default back to 1 later...
Edit: Fix several typos, I'm in a hurry
|
Now we have a lot of options ... I need to stop before featureism creeps in 😬 $ ./install.sh
Usage: install.sh [<options>] <cmd> [<args>]
Install and update Nerd Fonts [1] from the GitHub releases [2].
See `install.sh install --help` for details.
[1] https://www.nerdfonts.com/
[2] https://github.com/ryanoasis/nerd-fonts/releases
cmd:
help : show this help message
env : show environment
list : list released fonts
install : selectively install (or update) a font, a list of fonts or *all* fonts
remove : uninstall all Nerd Fonts
options:
-d, --dry do not execute, show what would happen (pair with -v)
-r, --release=TAG specify release tag, default is 'latest'
-s, --silent give no progress messages (resets verbose)
-v, --verbose increase verbosity level (up to 3 times)
-z, --zip use zip archive (needed for older releases)
-h, --help show this help message
--version show version information
required tools:
curl mktemp sed tar wcHmm, now that I read this, |
|
Nice nice I like this now, should we merge? |
|
It seems good for me! 👍 Much more complete! |
Adding support for install multiple font names using `install.sh` script. It accept a list of font names and install them in batch. This method increase functionality as it avoid install 1 or all only.
Remove bashisms while using bash arrays instead of strings. We can safely use strins as the fonts don't have whitespaces in names.
Typo.
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] I really hate setting VERBOSE=3 and then run the script instead of having a proper -v flag. [how] Process the options. Also centralizes the detection if help is requested, and if so supress contacting the Github servers with curl. That would not be used anyhow. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] If there is an unrecoverrable problem the temp directory will not be removed. [how] Use a trap to remove it in any case. Because we do not have a global variable with the directory name we evaluate it right away when setting the trap. Note that re-setting the trap would remove the old trap! We need to make certain we have not multiple temp directories with one script execution. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
We need the case-insensitive regex for this, and all other places that use shell regexes are fine with the change, so we do it script globally. Most of the changeset is a naming change from $1 to a concrete variable. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
We do not support two-word long options due to lazyness reasons. ok: -r v3.4.0 --release=v3.4.0 not supported: --release v3.4.0 Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Older releases have no xz archives. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] Some 'normal' information is displayed at verbosity level 1. The level 0 is in fact 'quiet'. I changed the default to 0 with adding the `-v` option. But immediately reconsidered and added the `-q` option to switch to silent mode, so the default can revert back to 1. But somehow forget to change the actual default. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] We want to have the same options available before and after the command. [how] Put argument processing into function. Call function twice, one initially and then once after the positional argument 'cmd' has been determined. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Also move version information only into actual command run. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
In case we want to add more options with arguments later on. Also reorder functions so that they are in the general 'section' given by the comments.
And also - unrelated - put startup information on error stream. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
d3b13d1 to
010d7ba
Compare
|
rebase on master, force push |
|
Cool! 😄 |

Description
Add the possibility of installing more than one font at time.
Requirements/Checklist
Issue number where discussion took place: #xxx
What does this Pull Request (PR) do?
This PR adds the possibility of installing more that one font at time, using the list of names provided
How should this be manually tested?
Call
./install.sh installwith the different combinations ofall,Font1 Font2or numberAny background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)