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.

Sign up for GitHub

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

Jump to bottom

Fix for MSYS2 #3

Merged
merged 4 commits into from
Oct 4, 2021
Merged

Fix for MSYS2 #3

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6effed3
Fix `sudo pacman: command not found` error
rashil2000 Oct 4, 2021
e82e934
Make it work with MSYS2
rashil2000 Oct 4, 2021
271b85c
Merge branch 'master' of github.com:joehillen/paruz into msys-fix
rashil2000 Oct 4, 2021
dfa041e
Remove special case for preview
rashil2000 Oct 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions paruz
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ if [[ -z $PARUZ ]]; then
elif ! command -v pacman >/dev/null 2>&1; then
echo "Neither paru nor pacman found. Is this Arch?" >&2
exit 1
elif [[ $EUID -eq 0 ]]; then
elif [[ $EUID -eq 0 ]] || [[ -f /usr/bin/msys-2.0.dll ]]; then
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need to special case msys2. It's fixed by your next commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no sudo on MSYS2. So we need to use just 'pacman'.

Copy link
Owner

Choose a reason for hiding this comment

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

Are you able to set environment variables? If so, try setting PARUZ=pacman

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm a bit confused. Isn't PARUZ being set to pacman here in the next line itself? I can set environment variables, but I'm unable to understand the need for it. (The Readme tells us that we explicitly set PARUZ when we want to use a different helper, but this is just pacman.)

Copy link
Owner

Choose a reason for hiding this comment

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

I'm trying to fix your problem without a special case for MSYS2. If you export PARUZ=pacman in your shell, then it won't do the root check or try sudo pacman.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm again trying too understand what's the harm in having a special case (genuine question). If it's an implementation problem, see my comment below.

Copy link
Owner

Choose a reason for hiding this comment

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

Does the environment variable work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it does.
But I find having to define PARUZ as 'pacman' slightly cumbersome, more so because there's already a fallback to 'pacman' in the script itself.
There's already a condition running in the if block there. Would it be considered really bad if we just put another OR check there? (Sorry again, I'm just trying to understand. I get that PRVW_CMD was slightly invasive as a platform-specific fix, so I removed it as per your instructions. But isn't this one really insignificant in comparison?)

Copy link
Owner

Choose a reason for hiding this comment

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

Adding an environment variable shouldn't be cumbersome. I have like 100 environment variables in my shell config. This scenario is exactly why I added the variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Paruz was made to handle paru and pacman (as I understand from the script). I would have happily set the environment variable if I was using some other helper (as the Readme tells us).

I will remove this condition if this feels really unnecessary.

PARUZ=pacman
else
PARUZ='sudo pacman'
Expand Down Expand Up @@ -83,7 +83,7 @@ __paruz_list |
fzf \
--multi \
--ansi \
--preview="'${BASH_SOURCE[0]}' __fzf_preview {1}" |
--preview="/usr/bin/env bash '${BASH_SOURCE[0]}' __fzf_preview {1}" |
readarray -t PICKS

if [[ ${#PICKS[@]} -eq 0 ]]; then
Expand Down