Skip to content

grive-sync: Remove "$HOME" subdirectory requirement - #385

Open
KreAch3R wants to merge 1 commit into
vitalif:masterfrom
KreAch3R:0.5.3-no-home-dir
Open

KreAch3R wants to merge 1 commit into
vitalif:masterfrom
KreAch3R:0.5.3-no-home-dir

Conversation

@KreAch3R

@KreAch3R KreAch3R commented Aug 4, 2023

Copy link
Copy Markdown

Check #383

@vitalif

vitalif commented Aug 4, 2023

Copy link
Copy Markdown
Owner

Hi. Just remove it, don't comment it out
This script was contributed by someone here in PRs so I have no idea why it was limited to $HOME

@KreAch3R

KreAch3R commented Aug 4, 2023

Copy link
Copy Markdown
Author

Hi. Just remove it, don't comment it out This script was contributed by someone here in PRs so I have no idea why it was limited to $HOME

Hello, ah, ok. Done. If you are interested in clean code, I can try to force push another clean commit but I'm not sure how the PR is going to handle it.

Comment thread systemd/grive-sync.sh
DIRECTORY=$(systemd-escape --unescape -- "$2")

if [[ -z "$DIRECTORY" ]] || [[ ! -d "$DIRECTORY" ]] ; then
echo "Need a directory name in the current users home directory as second argument. Aborting."

@jankatins jankatins Aug 6, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The test is alright (assuming -z tests for "not defined"), but the msg should be "need a directory as either absolute path or relative to $HOME"

@KreAch3R KreAch3R Aug 6, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You were right. I took a better look and what exactly was happening and it was all about a "/"....

@KreAch3R
KreAch3R force-pushed the 0.5.3-no-home-dir branch from ed54894 to a9775e5 Compare August 6, 2023 19:05
@jankatins

Copy link
Copy Markdown

Right now I feel that the original test is better suited: if I would configure the script to sync opt/whatever, I would be very suprised if it would use /opt/whatever and not ${HOME}/opt/whatever (for the former, i would want to configure /opt/whatever with a leading slash). For that reason, I would go with just changing the help message:

if [[ -z "${DIRECTORY}" ]] || [[ ! -d "${DIRECTORY}" ]] ; then
	echo "Need a directory name (absolute or relative to the current users home directory) as second argument. Got ${DIRECTORY}. Aborting."
	exit 1
fi

I've actually adjusted that in my local version of the script which I started with systemctl --user start $(systemd-escape --template=grive@.service -- "${HOME}/.gdrive"), so an absolute path.

@jankatins

Copy link
Copy Markdown

See #389 for my version

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