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

Configure global installation #62

Open
wants to merge 56 commits into
base: master
Choose a base branch
from

Conversation

hunter-richardson
Copy link
Contributor

@hunter-richardson hunter-richardson commented Oct 28, 2022

#50

  • Added an installation script for installing/configuring fundle globally (includes a flag to restrict user installations)
  • Added functions to functions/fundle.fish for installing and updating global scripts
  • Added completions for these functions to completions/fundle.fish
  • Included global plugins in fundle list printout (disabled the list verbosity flag)
  • Updated README with new functionality

Also (this can be rolled back if necessary, since it isn't related):

  • Optimized/cleaned previous code
  • Use builtin and command where possible to avoid invoking user aliases
  • Replaced echo with printf where possible, since echo is more expensive
  • Use fish builtin argparse to implement --argument-based functionality
  • Deleted unreachable code

Please note: I have not been able to test this functionality, since I don't currently have a personal computer (thanks, ex-wife) and I can't run fish at work.

@hunter-richardson hunter-richardson marked this pull request as draft October 28, 2022 23:04
@BarbzYHOOL
Copy link
Contributor

I may test this if you ask me to, but not immediately, just hit me up, i'll get back to you

@hunter-richardson hunter-richardson marked this pull request as ready for review October 31, 2022 14:32
@hunter-richardson
Copy link
Contributor Author

hunter-richardson commented Oct 31, 2022

@BarbzYHOOL I would appreciate that! It should be ready now.

@BarbzYHOOL
Copy link
Contributor

@hunter-richardson I was gonna test, i was reading the new readme (saw few typos + link to the obsolete "tuvistavie" github name) and the changed files, and then i thought "but how do i install it??"
I am not sure how to test all of this because the install script links to the current fundle.fish and it's not yours

plz give me a simple way to test this (also i haven't tried a PR for more than 2 years, i'm a bit rusty)

@hunter-richardson
Copy link
Contributor Author

hunter-richardson commented Nov 3, 2022

@BarbzYHOOL LMAO

I wrote the README changes to with the idea in mind that @danhper would merge it. So the link to my new installation script is broken until that happens.

You should be able to just follow the installation instructions, but replace the broken link with the working one.

EDIT: I just realized that simply using my installer also wouldn't grab my new changes to fundle itself, so:

wget https://raw.githubusercontent.com/hunter-richardson/fundle-global/master/install-fundle-global.fish
nano ./install-fundle-global.fish
  # replace "https://raw.githubusercontent.com/tuvistavie/fundle/master/$i/fundle.fish" with "https://raw.githubusercontent.com/hunter-richardson/fundle-global/master/$i/fundle.fish"
  # Ctrl-X y Enter
fish -c "source ./install-fundle-global.fish"
  # optionally with --restrict-user-plugins
rm -rf ./install-fundle-global.fish

@BarbzYHOOL
Copy link
Contributor

Couldn't install, here are the commands and the output

hastebin.com/danexexika.yaml

@BarbzYHOOL
Copy link
Contributor

@hunter-richardson in case you didn't see my reply

@BarbzYHOOL
Copy link
Contributor

the hastebin has expired sadly, but it was full of errors, i don't remember, couldn't install it

@hunter-richardson
Copy link
Contributor Author

hunter-richardson commented Dec 13, 2022

@BarbzYHOOL
Sorry it took me so long to get back with you! Work has been a little crazy while I transitioned back to the home-office.

I was able to scan your error log briefly; if I remember correctly, the first few lines looked like invalid permissions. Were you a logged-in as a sudoer on your machine?

EDIT: My bad, I just realized that even if you are a sudoer, you still have to sudo as root to access the /root/.config directories. Hopefully it works now!

@BarbzYHOOL
Copy link
Contributor

BarbzYHOOL commented Dec 31, 2022

tried the commands you posted on november 3rd, nothing worked, I don't recall the errors but I think it was permission related

@danhper
Copy link
Owner

danhper commented Jan 4, 2023

Hi and thanks a lot for working on this.
I think there are currently a few issues with the install script.

First, something like this will not work:

sudo echo foo > /some/protected/file

because only echo will be sudoed and the redirection will be done by the current user, so unless it already has the permissions, this will fail.
A common work around is to replace it by

echo foo | sudo tee /some/protected/file > /dev/null

so that the file is actually written to using sudo. tee -a can replace >>.
There also seem to be a few sudo missing, e.g. for grep or chmod.
The argparse currently does not work with older version of fish (e.g. 3.1.0). The 'restrict_user_plugins'part should be changed to'r-restrict_user_plugins'. I also believe that $argsshould be$argv`.

@hunter-richardson
Copy link
Contributor Author

@danhper Good catch, and thanks for the assist! I have corrected these errors, although I don't fully understand the distinction with the change to argparse.

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.

3 participants