Skip to content

Add support for XDG Base Directory spec #49

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

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

Add support for XDG Base Directory spec #49

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
971a173
Add support for XDG Base Directory spec
Jan 17, 2021
462bfd9
Implement changes as suggest by @akinomyoga
Jan 17, 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
17 changes: 15 additions & 2 deletions fzf-marks.plugin.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,25 @@

command -v fzf >/dev/null 2>&1 || return

PROGRAM_NAME="${PROGRAM_NAME:-fzf-marks}"

CONFIG_DIR="${XDG_CONFIG_HOME:-$HOME/.config}/$PROGRAM_NAME" #currently unused, but in the future can be used to load (and store?) an .env file from to more easily switch between multiple configuration
DATA_DIR="${XDG_DATA_HOME:-$HOME/.local/share}/$PROGRAM_NAME"
LEGACY_FZF_MARKS_FILE="${HOME}/.fzf-marks"
Comment thread
FiXato marked this conversation as resolved.
Outdated

if [[ -z ${FZF_MARKS_FILE-} ]] ; then
FZF_MARKS_FILE=${HOME}/.fzf-marks
FZF_MARKS_FILE="${DATA_DIR}/default_bookmarks.fzf-marks"
fi

if [[ ! -f ${FZF_MARKS_FILE} ]]; then
if [[ ! -f "${FZF_MARKS_FILE}" ]]; then

@akinomyoga akinomyoga Jan 17, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These quotes (") are not needed in the conditional construct [[ ... ]]. In the conditional construct, arguments are not subject to word splitting and pathname expansions.

Similarly, the right-hand side of the variable assignments is neither subject to word splitting and pathname expansions, so the quotes in FZF_MARKS_FILE="${DATA_DIR}/default_bookmarks.fzf-marks" are also unneeded.

@FiXato FiXato Jan 17, 2021

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.

I guess I'm just used to using double quotes where possible to prevent word splitting. :)
Is there a downside to using quotes inside the conditional construct?

@akinomyoga akinomyoga Jan 18, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually, I think there is no downside to quote the parameter expansion, so yeah, this is a matter of preference. One of the reasons that the keyword [[ is introduced as a variant of the command [ is to resolve quotation problems in [, so I just felt it natural to quote the arguments only when necessary.

@urbainvaes urbainvaes Jan 18, 2021

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@akinomyoga makes a good point. At some point in the future, I'll go through the codes and make sure we are consistent everywhere. Same with $FZF_MARKS_FILE vs ${FZF_MARKS_FILE}. Since the quotes, and in the latter case the curly brackets, are unnecessary, we could do away with them, but for this pull request this does not matter much.

# Fallback support for the previous default location of the bookmarks file.
if [[ -f "$LEGACY_FZF_MARKS_FILE" ]]; then
echo "Your FZF_MARKS_FILE is still in its legacy location of $LEGACY_FZF_MARKS_FILE; you might want to consider moving it to "$FZF_MARKS_FILE" so it follows the XDG standard, or explicitly set FZF_MARKS_FILE='$LEGACY_FZF_MARKS_FILE'." >&2

@urbainvaes urbainvaes Jan 18, 2021 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Are the quotes around "$FZF_MARKS_FILE" necessary here? Since there are no quotes around $LEGACY_FZF_MARKS_FILE, it might be good to remove them for consistency?

@FiXato FiXato Jan 18, 2021

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.

I believe I already changed that based on @akinomyoga 's suggestions :)
See 462bfd9 :)

@urbainvaes urbainvaes Jan 22, 2021

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good, many thanks! :)

FZF_MARKS_FILE="$LEGACY_FZF_MARKS_FILE"
else
[[ ! -d "$DATA_DIR" ]] && mkdir "$DATA_DIR" && chmod 700 "$DATA_DIR"
Comment thread
FiXato marked this conversation as resolved.
Outdated
touch "${FZF_MARKS_FILE}"
fi
fi

if [[ -z ${FZF_MARKS_COMMAND-} ]] ; then
Expand Down