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

NerdFonts installer script added and consequential change to tab_data TOML file. #446

Closed
wants to merge 4 commits into from
Closed

NerdFonts installer script added and consequential change to tab_data TOML file. #446

wants to merge 4 commits into from

Conversation

fam007e
Copy link

@fam007e fam007e commented Sep 17, 2024

Type of Change

  • New feature
  • Bug fix
  • Documentation Update
  • Refactoring
  • Hotfix
  • Security patch
  • UI/UX improvement

Description

The NerdFonts installer script has been added, allowing users to select and install a variety of NerdFonts through a command-line interface. This feature ensures that the required fonts are downloaded and installed in the user’s ~/.local/share/fonts directory. Additionally, the font cache is updated to reflect the changes.

A consequential change has been made to the tab_data TOML file to integrate the new functionality and ensure smooth operation across the platform.

Key Features:

  • Users can select multiple fonts from a displayed list with scroll functionailty via less.
  • Fonts are downloaded from the latest NerdFonts GitHub release.
  • The font cache is automatically updated after installation.

Testing

  • Ran the script to verify correct downloading and installation of multiple fonts.
  • Verified that the font cache updates properly and fonts are available for use.
  • Tested the TOML changes for integration with the current configuration.

Impact

This feature will allow users to easily install and manage NerdFonts, reducing manual download and installation time. There are no new dependencies outside the standard utilities (curl, tar,less) except that of fc-cache from fontconfig.

Issue related to PR

Additional Information

No additional information at this time.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no errors/warnings/merge conflicts.

@adamperkowski
Copy link
Collaborator

adamperkowski commented Sep 17, 2024

No way to scroll here when the terminal with linutil is smaller

image

Comment on lines 122 to 127
curl -sSLo "$HOME/tmp/$font_name.tar.xz" "https://github.com/ryanoasis/nerd-fonts/releases/latest/download/$font_name.tar.xz"

# Extract and install the font
mkdir -p ~/.local/share/fonts
tar -xf "$HOME/tmp/$font_name.tar.xz" -C "$HOME/.local/share/fonts"
rm "$HOME/tmp/$font_name.tar.xz"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
curl -sSLo "$HOME/tmp/$font_name.tar.xz" "https://github.com/ryanoasis/nerd-fonts/releases/latest/download/$font_name.tar.xz"
# Extract and install the font
mkdir -p ~/.local/share/fonts
tar -xf "$HOME/tmp/$font_name.tar.xz" -C "$HOME/.local/share/fonts"
rm "$HOME/tmp/$font_name.tar.xz"
curl -sSLo "/tmp/$font_name.tar.xz" "https://github.com/ryanoasis/nerd-fonts/releases/latest/download/$font_name.tar.xz"
# Extract and install the font
mkdir -p ~/.local/share/fonts
tar -xf "/tmp/$font_name.tar.xz" -C "$HOME/.local/share/fonts"
rm "/tmp/$font_name.tar.xz"

Copy link
Contributor

@nnyyxxxx nnyyxxxx Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamperkowski this needs escalation_tool you cannot curl a file inside of the root file system without root permissions nvm tested it and it works, learn something new everyday

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nnyyxxxx I'm pretty sure you're wrong this time. The default /tmp permissions are drwxrwxrwt. The t at the end means that only the owner of a file or the root user can modify.
image

checkCommandRequirements "tar"

# Download the font
curl -sSLo "$HOME/tmp/$font_name.tar.xz" "https://github.com/ryanoasis/nerd-fonts/releases/latest/download/$font_name.tar.xz"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curl: (3) URL rejected: Malformed input to a URL function

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tar and curl are preloaded so what's the issue here.
I could not reproduce your error: curl: (3) URL rejected: Malformed input to a URL function

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tar and curl are preloaded so what's the issue here. I could not reproduce your error: curl: (3) URL rejected: Malformed input to a URL function

Then you did not test it, "TMP" a folder inside of the root file system does not exist inside of the users home directory

Copy link
Collaborator

@adamperkowski adamperkowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename nerdfonts-installer.sh to nerdfonts.sh

tabs/system-setup/tab_data.toml Outdated Show resolved Hide resolved
@adamperkowski
Copy link
Collaborator

The script should also check if fontconfig is installed.

@adamperkowski
Copy link
Collaborator

I'm sorry but I cannot approve 100% LLM generated PRs. You need to rewrite the code yourself.

Co-authored-by: Adam Perkowski <adas1per@protonmail.com>
@fam007e
Copy link
Author

fam007e commented Sep 17, 2024

No way to scroll here when the terminal with linutil is smaller

image

I used my 15" laptop and I also think even if it's 14" or 13" its okay so how can you make it posix compliant and fit it in within the scope i.e. having to add scroll functionality is beyond my level of understanding... So forgive me I can NOT accomplish that task.

@adamperkowski
Copy link
Collaborator

how can you make it posix compliant and fit it in within the scope i.e. having to add scroll functionality is beyond my level of understanding...

You can use printf with read (#282) or just craft a string and more it.

@fam007e
Copy link
Author

fam007e commented Sep 17, 2024

The script should also check if fontconfig is installed.

Shouldn't I add check for fc-cache command instead.

@adamperkowski
Copy link
Collaborator

Shouldn't I add check for fc-cache command instead.

You should check if fontconfig exists in the system. If not, install it.

You can do command_exists fc-cache and if not, install fontconfig.

@fam007e
Copy link
Author

fam007e commented Sep 17, 2024

Shouldn't I add check for fc-cache command instead.

You should check if fontconfig exists in the system. If not, install it.

You can do command_exists fc-cache and if not, install fontconfig.

I think the issue with that is if debian has other utility which installs fontconfig.

…oviding the ability to scroll both forwards and backwards through the list of fonts
@fam007e
Copy link
Author

fam007e commented Sep 17, 2024

Shouldn't I add check for fc-cache command instead.

You should check if fontconfig exists in the system. If not, install it.

You can do command_exists fc-cache and if not, install fontconfig.

May be the "and if not" part be actually implemented in common-script rather than in individual cases like this... 🤔

@adamperkowski
Copy link
Collaborator

I think the issue with that is if debian has other utility which installs fontconfig.

You can use

case $PACKAGER in
apt|nala)
    # debian stuff
    ;;
...

@adamperkowski
Copy link
Collaborator

adamperkowski commented Sep 17, 2024

@fam007e Draft this with #447 & #448 for now. Chris is supposed to be streaming soon.

@fam007e
Copy link
Author

fam007e commented Sep 17, 2024

@fam007e Draft this and #447 for now. Chris is supposed to be streaming soon.

I had another one of using cli to setup ssh to github along with nerdfonts and bootloader-switcher. These three were made into 1 PR. However, one maintainer suggested to do three separate so I had to use Claude LLM to write the PRs' text as I am quite busy this week. Sorry about all that. 🙏

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.

Nerd Fonts Installer Feature Request #160 Add option to install Nerd fonts in Linutil
3 participants