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

Merge Linux and Mac Install Script + Fixes + Messages #64

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

Conversation

abeljim
Copy link

@abeljim abeljim commented Nov 11, 2023

Fixes #63
Install script is now a single file for mac and Linux.
Handles XDG_DATA_HOME if available.
Prints out progress messages for the user.
Updated ReadMe.

# copy all fonts from ./otf to ~/.local/share/fonts
cp ./fonts/otf/* ~/.local/share/fonts
cp ../fonts/otf/* ~/.local/share/fonts
Copy link
Contributor

Choose a reason for hiding this comment

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

This depends on the current working directory when the script is called.

Maybe better use a path relative to the script location, then one can call the script from any directory?

Suggested change
cp ../fonts/otf/* ~/.local/share/fonts
# Get script directory
sd=$(cd -- "$(dirname "$0")" >/dev/null 2>&1 || exit; pwd)
# copy all fonts from ./otf to ~/.local/share/fonts
cp "${sd}/../fonts/otf/*" ~/.local/share/fonts

(Well, something to that effect)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this has still not been addressed.

You could call install.sh from any current working directory. But all the paths in the script are relative to the current working directory instead of relative to the script directory. That in effect means one needs to change into the concrete directory that the author of the script envisioned.

For example

cd ~/git/monaspace
util/install.sh

cd ~/git/monaspace/util
./install.sh

cd /tmp
~/git/monaspace/util/install.sh

They should imho all work.

For the script to assemble the source path of the font files it needs the absolute path of the script itself and from then on it could relative walk to the fonts directory. Thus the code above.

@Finii
Copy link
Contributor

Finii commented Nov 11, 2023

In principle it could use the XDG_DATA_HOME also, instead of assuming where share/fonts is, for people who have a different directory layout.

# Get target root directory
if [[ $(uname) == 'Darwin' ]]; then
  # MacOS
  sys_share_dir="/Library"
  usr_share_dir="$HOME/Library"
  font_subdir="Fonts"
else
  # Linux
  sys_share_dir="/usr/local/share"
  usr_share_dir="$HOME/.local/share"
  font_subdir="fonts"
fi
if [ -n "${XDG_DATA_HOME}" ]; then
  usr_share_dir="${XDG_DATA_HOME}"
fi

Code snippet that allows Linux and MacOS installs, system-wide or user

https://wiki.archlinux.org/title/XDG_Base_Directory

@Finii
Copy link
Contributor

Finii commented Nov 11, 2023

Better fix is #66

@Finii Finii mentioned this pull request Nov 11, 2023
@abeljim abeljim changed the title Updated linux install script to fix readme and folder not existing Merge Linux and Mac Install Script + Fixes + Messages Nov 11, 2023
@abeljim abeljim requested a review from Finii November 11, 2023 18:33
@abeljim
Copy link
Author

abeljim commented Nov 11, 2023

Updated script based on feedback. See first post for more info.

@lucaspar
Copy link

lucaspar commented Nov 12, 2023

I'm seeing these PRs after writing my own modifications to this installation script too 😅

IMO Monaspace should have its own directory in fonts e.g.: ${XDG_DATA_HOME}/fonts/Monaspace/, as over 200 files are copied there by this script.

@idan
Copy link
Contributor

idan commented May 9, 2024

Hi there! This seems way nicer than our crappy install scripts, but I don't want to hold up releasing v1.101 for it. Going to figure this out for 1.2, alongside #89. Stay tuned!

# Get target root directory
if [[ $(uname) == 'Darwin' ]]; then
# MacOS
sys_share_dir="/Library"
Copy link

Choose a reason for hiding this comment

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

This is never used.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the code comes from my snippet.

That variable would be used if the script allows system-wide installation.
As it does not (yet) you are right it's unused/

echo "Detected MacOS. Setting directories accordingly."
else
# Linux
sys_share_dir="/usr/local/share"
Copy link

Choose a reason for hiding this comment

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

same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

Linux Install Script Issues
5 participants