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

Now "AppMan" prompts you to enable ~/.local/bin in $PATH, if not in place #707

Merged
merged 10 commits into from
Jun 23, 2024

Conversation

Samueru-sama
Copy link
Contributor

I did some limited testing, but I'm not a bash user so let me know if I need to change anything.

I removed the patching of the .bashrc because as far as I know it isn't needed and can actually cause problems like I mentioned in the issue.

closes #705 #706

@Samueru-sama
Copy link
Contributor Author

wait my read isn't working lol

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

I'd like to remove completelly this patch, $PATH order is something that the user should choose.

@Samueru-sama
Copy link
Contributor Author

I'd like to remove completelly this patch, $PATH order is something that the user should choose.

We still need the warning, because otherwise if $HOME/.local/bin isn't in path that would result in appimages that can't be launched from the terminal.

What I did will echo a warning with the optional question to patch the files. However right now the behaviour is that appman will not continue (exit 1) if the user says no.

@Samueru-sama
Copy link
Contributor Author

Oh wait I changed something I shouldn't have changed

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

@Samueru-sama we have a guide for AppMan https://github.com/ivan-hc/AM#how-to-install-appman

the behaviour of local configuration files shoul not be something we would decide.

also, what @rvenutolo said is that we could overcome this in other ways #705

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

Let me see your function

function _patch_bashrc_and_profile() {
	if ! echo "$PATH" | grep -q "$BINDIR"; then
		read -rp "Warming: \"${XDG_BIN_HOME:-$HOME/.local/bin}\" is not in PATH, would you like appman to patch your .profile or .bash_profile? (Y/n): " yn
		if ! echo "$yn" | grep -i '^n' >/dev/null 2>&1; then
			if test -f "$HOME/.profile"; then
				printf '\n%s\n' 'export PATH="$PATH:${XDG_BIN_HOME:-$HOME/.local/bin}"' >> "$HOME/.profile"
			elif test -f "$HOME/.bash_profile"; then
				printf '\n%s\n' 'export PATH="$PATH:${XDG_BIN_HOME:-$HOME/.local/bin}"' >> "$HOME/.bash_profile"
			fi
			echo "You might need to resource your .profile / .bash_profile or reboot for these changes to take effect"
		else
			exit 1
		fi
	fi
}

web view sucks, I still have not understand how to split the views.

Your function looks good.

@Samueru-sama
Copy link
Contributor Author

@Samueru-sama we have a guide for AppMan https://github.com/ivan-hc/AM#how-to-install-appman

the behaviour of local configuration files shoul not be something we would decide.

also, what @rvenutolo said is that we could overcome this in other ways #705

Alright if you don't want it, simply remove the if statements that check for "$HOME/.profile" and "$HOME/.bash_profile"

I don't agree with not having the option to do the patching, you have to be aware that there are people that have no idea what PATH is and won't bother trying to fix it if appman tells them that their $HOME/.local/bin is missing from it.

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

Something seems to be too strange, if an user using AppMan uses AppMan from ~/.local/bin... why should not have this in $PATH?

@Samueru-sama
Copy link
Contributor Author

Something seems to be too strange, if an user using AppMan uses AppMan from ~/.local/bin... why should not have this in $PATH?

Sorry what?

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

Something seems to be too strange, if an user using AppMan uses AppMan from ~/.local/bin... why should not have this in $PATH?

Sorry what?

if an user is able to use something like appman from ~/.local/bin, why should need to add ~/.local/bin in $PATH?

should it be already in $PATH?

@Samueru-sama
Copy link
Contributor Author

should it be already in $PATH?

No! that's the big issue.

It actually happened to me when I started using linux, I couldn't get some scripts to work and it was because arch doesn't bother to add $HOME/.local/bin to PATH.

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

I think its better to write this in the installation guide and completelly remove that function from APP-MANAGER

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

should it be already in $PATH?

No! that's the big issue.

It actually happened to me when I started using linux, I couldn't get some scripts to work and it was because arch doesn't bother to add $HOME/.local/bin to PATH.

I agree to add it if not in place, OK.

@ivan-hc
Copy link
Owner

ivan-hc commented Jun 22, 2024

Discord

@ivan-hc ivan-hc changed the title Update APP-MANAGER Now "AppMan" prompts you to enable ~/.local/bin in $PATH, if not in place Jun 23, 2024
@ivan-hc ivan-hc changed the base branch from dev to main June 23, 2024 01:20
@ivan-hc ivan-hc merged commit c28a77c into ivan-hc:main Jun 23, 2024
3 checks passed
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.

2 participants