Support for XDG Base Directory Specification #1282

mark64 · 2018-05-06T07:09:42Z

This makes it possible to keep your home directory free from fzf dotfiles. One thing to note is that this does not handle the case where a ~/.fzf.bash file already exists. I can include that if desired.

junegunn · 2018-05-06T10:19:40Z

install

@@ -9,6 +9,13 @@ update_config=2
 binary_arch=
 allow_legacy=
 shells="bash zsh fish"
+config_dir=~/.config/fzf


Don't want to suddenly change the default directory after 4 and a half years. Please change it back to ~.
(Also note that Travis CI build is failing because of the change.)

I reverted the default directory to ~/, but also added a check to see if the user manually created a ~/.config/fzf directory, which I hope is fine since some users want their dotfiles in ~/.confg but don't want to set extra environment variables.

Sounds good to me.

junegunn · 2018-05-06T10:20:57Z

install

+fish_dir=~/.config/fish
+if [[ -v XDG_CONFIG_HOME ]] && [ ! -z $XDG_CONFIG_HOME ] && [ -d $XDG_CONFIG_HOME ]; then
+    config_dir=$XDG_CONFIG_HOME/fzf
+    fish_dir=$XDG_CONFIG_HOME/fish


We use 2-space indentation throughout the code.

junegunn · 2018-05-06T10:21:21Z

uninstall

+fish_dir=~/.config/fish
+if [[ -v XDG_CONFIG_HOME ]] && [ ! -z $XDG_CONFIG_HOME ] && [ -d $XDG_CONFIG_HOME ]; then
+    config_dir=$XDG_CONFIG_HOME/fzf
+    fish_dir=$XDG_CONFIG_HOME/fish


Indentation.

junegunn · 2018-05-06T10:24:21Z

install

@@ -9,6 +9,13 @@ update_config=2
 binary_arch=
 allow_legacy=
 shells="bash zsh fish"
+config_dir=~/.config/fzf
+fish_dir=~/.config/fish
+if [[ -v XDG_CONFIG_HOME ]] && [ ! -z $XDG_CONFIG_HOME ] && [ -d $XDG_CONFIG_HOME ]; then


-z seems unnecessary. We can write [[ -v XDG_CONFIG_HOME ]] && [[ -d "$XDG_CONFIG_HOME" ]]

-z does seems unnecessary, but the XDG base dir spec allows for that env var to be empty, in which case it should be ignored and code should use a default directory (normally ~/.config).

From here

$XDG_CONFIG_HOME defines the base directory relative to which user specific configuration files should be stored. If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.

Good point, thanks.

junegunn · 2018-05-08T03:00:45Z

install

@@ -9,6 +9,15 @@ update_config=2
 binary_arch=
 allow_legacy=
 shells="bash zsh fish"
+config_base=~/.
+fish_dir=~/.config/fish


No slash at the end. Actually, I don't like requiring $fish_dir to have a trailing slash, and I would much prefer $fish_dir/functions over ${fish_dir}functions as it's cleaner and less bug-prone. I can see that it's necessary for $config_base, so we have either ~/.fzf.bash or ~/.config/fzf/fzf.bash but we don't have to do the same for $fish_dir. Maybe we can rename $config_base to $config_prefix, to make the difference more explicit ("prefix" vs. "dir").

junegunn · 2018-05-13T07:47:23Z

Thanks for the update, there's one more problem.

Before the patch:

Update /Users/jg/.bashrc:
  - [ -f ~/.fzf.bash ] && source ~/.fzf.bash
    - Already exists: line #559

Update /Users/jg/.zshrc:
  - [ -f ~/.fzf.zsh ] && source ~/.fzf.zsh
    - Already exists: line #86

and after the patch

Update /Users/jg/.bashrc:
  - [ -f /Users/jg/.fzf.bash ] && source /Users/jg/.fzf.bash
    + Added

Update /Users/jg/.zshrc:
  - [ -f /Users/jg/.fzf.zsh ] && source /Users/jg/.fzf.zsh
    + Added

You see the difference? And I end up with both versions of the line in my .bashrc

[ -f ~/.fzf.bash ] && source ~/.fzf.bash

[ -f /Users/jg/.fzf.bash ] && source /Users/jg/.fzf.bash

This is not desirable, and I have noticed that most users prefer the former as it can be used with different usernames.

The following patch seems to fix the problem. Please let me know if you have a better idea.

diff --git a/install b/install
index d131c82..ee851ad 100755
--- a/install
+++ b/install
@@ -9,14 +9,14 @@ update_config=2
 binary_arch=
 allow_legacy=
 shells="bash zsh fish"
-config_prefix=~/.
+config_prefix="~/."
 fish_dir=~/.config/fish
 if [[ -v XDG_CONFIG_HOME ]] && [[ ! -z $XDG_CONFIG_HOME ]] && [[ -d $XDG_CONFIG_HOME ]]; then
   config_prefix=$XDG_CONFIG_HOME/fzf/
   fish_dir=$XDG_CONFIG_HOME/fish
   mkdir -p $config_prefix
 elif [[ -d ~/.config/fzf ]]; then
-  config_prefix=~/.config/fzf/
+  config_prefix="~/.config/fzf/"
 fi
 
 help() {
@@ -262,7 +262,8 @@ for shell in $shells; do
     fzf_key_bindings="# $fzf_key_bindings"
   fi
 
-  cat > $src << EOF
+  eval "src=\"$src\""
+  cat > "$src" << EOF
 # Setup fzf
 # ---------
 if [[ ! "\$PATH" == *$fzf_base/bin* ]]; then

mark64 · 2018-05-14T01:04:13Z

Good point, thanks for the patch!

junegunn · 2018-05-14T08:48:44Z

Thanks, one more thing though, can you make the code compatible with the paths with whitespaces? We didn't have the problem before, but now we have to consider such unusual cases.

export XDG_CONFIG_HOME="/tmp/foo bar"
mkdir -p "$XDG_CONFIG_HOME"
./install --all

junegunn · 2018-05-18T09:04:29Z

So we're disallowing $XDG_CONFIG_HOME with spaces. Is it officially forbidden by the spec? Or are we just ignoring the case to simplify the code?
If a user temporarily sets $XDG_CONFIG_HOME just before running the script, the added line will refer to it, but the variable may not be available on other sessions, which can be a source of confusion. One way to avoid such a situation is to provide a default value for $XDG_CONFIG_HOME; i.e. config_prefix='${XDG_CONFIG_HOME:-~/.config}/fzf/', but this can be an overkill. I'm a bit torn on this idea.
Need to quote config_prefix in uninstall script
Let's say a user already installed fzf without $XDG_CONFIG_HOME, then decided to use $XDG_CONFIG_HOME. If he reruns the install script, the configuration file will have two lines for loading fzf.

[ -f ~/.fzf.bash ] && source ~/.fzf.bash

[ -f $XDG_CONFIG_HOME/fzf/fzf.bash ] && source $XDG_CONFIG_HOME/fzf/fzf.bash

We can consider avoiding XDG version if ~/.fzf.bash already exists (which means this is not a fresh-install).

mark64 · 2018-05-19T03:31:26Z

Not forbidden, my bad. I assumed you were referring to the possibility of specifying 2, space-separated directory, not an actual directory with spaces in the name. I now quote the mkdir line properly.
I didn't add it in the latest commit, but I could do config_prefix="\${XDG_CONFIG_HOME:-${XDG_CONFIG_HOME}}/fzf/". It is quite verbose and a little ugly
Done.
Done.

junegunn · 2018-05-20T03:07:17Z

Thanks, but the space issue still lurks in a number of places.

rm -f ~/.fzf.bash
export XDG_CONFIG_HOME="/tmp/foo bar"
mkdir -p "$XDG_CONFIG_HOME"
./install --all

Downloading bin/fzf ...
  - Already exists
  - Checking fzf executable ... 0.17.3

Generate ${XDG_CONFIG_HOME:-/tmp/foo bar}/fzf/fzf.bash ... ./install: line 267: $src: ambiguous redirect
OK
Generate ${XDG_CONFIG_HOME:-/tmp/foo bar}/fzf/fzf.zsh ... ./install: line 267: $src: ambiguous redirect
OK
Update fish_user_paths ... OK
./install: line 296: [: /tmp/foo: binary operator expected
Symlink /tmp/foo bar/fish/functions/fzf_key_bindings.fish ... ln: /tmp/foo bar/fish/functions/fzf_key_bindings.fish: No such file or directory
Failed

Update /Users/jg/.bashrc:
  - [ -f ${XDG_CONFIG_HOME:-/tmp/foo bar}/fzf/fzf.bash ] && source ${XDG_CONFIG_HOME:-/tmp/foo bar}/fzf/fzf.bash
    + Added

Update /Users/jg/.zshrc:
  - [ -f ${XDG_CONFIG_HOME:-/tmp/foo bar}/fzf/fzf.zsh ] && source ${XDG_CONFIG_HOME:-/tmp/foo bar}/fzf/fzf.zsh
    + Added

Create /tmp/foo bar/fish/functions/fish_user_key_bindings.fish:
    function fish_user_key_bindings
./install: line 353: /tmp/foo bar/fish/functions/fish_user_key_bindings.fish: No such file or directory
      fzf_key_bindings
./install: line 353: /tmp/foo bar/fish/functions/fish_user_key_bindings.fish: No such file or directory
    end
./install: line 353: /tmp/foo bar/fish/functions/fish_user_key_bindings.fish: No such file or directory

Finished. Restart your shell or reload config file.
   source ~/.bashrc  # bash
   source ~/.zshrc   # zsh
   fzf_key_bindings  # fish

Use uninstall script to remove fzf.

junegunn · 2018-05-20T03:10:32Z

Also note that [ -f $XDG_CONFIG_HOME/fzf/fzf.bash ] needs quoting if $XDG_CONFIG_HOME contains whitespaces.

export XDG_CONFIG_HOME="/tmp/foo bar"
mkdir -p "$XDG_CONFIG_HOME"
touch $XDG_CONFIG_HOME/baz
[ -f $XDG_CONFIG_HOME/baz ] && echo 1
# bash: [: /tmp/foo: binary operator expected

Related junegunn#1035

[ -f "$XDG_CONFIG_HOME"/fzf/fzf.bash ] && source "$XDG_CONFIG_HOME"/fzf/fzf.bash

junegunn · 2018-05-28T03:53:43Z

Sorry for the delay, I just pushed commits that should fix the whitespace issue.

Tested with the following script:

rm -f ~/.fzf.bash
export XDG_CONFIG_HOME="/tmp/foo bar"
mkdir -p "$XDG_CONFIG_HOME"
./install --all
./uninstall

unset XDG_CONFIG_HOME
rm -rf ~/.config/fzf
./install --all
./uninstall

mkdir -p ~/.config/fzf
./install --all
./uninstall

Now, bashrc can have three versions of the line depending on the environment variables of the shell that runs the install script.

[ -f ~/.fzf.bash ] && source ~/.fzf.bash
[ -f "$XDG_CONFIG_HOME"/fzf/fzf.bash ] && source "$XDG_CONFIG_HOME"/fzf/fzf.bash
[ -f ~/.config/fzf/fzf.bash ] && source ~/.config/fzf/fzf.bash

And I'm not sure if this is a good decision as it can be confusing. Maybe we should ditch the third case?

junegunn · 2018-05-28T04:27:09Z

Another option would be to combine the latter two:

[ -f "${XDG_CONFIG_HOME:-$HOME/.config}"/fzf/fzf.bash ] && source "${XDG_CONFIG_HOME:-$HOME/.config}"/fzf/fzf.bash

Doesn't look good though.

mark64 · 2018-05-28T05:16:49Z

The third case should be handled, and though its verbose I think your suggestion to do [ -f "${XDG_CONFIG_HOME:-$HOME/.config}"/fzf/fzf.bash ] && source "${XDG_CONFIG_HOME:-$HOME/.config}"/fzf/fzf.bash is the best way to cover 2 and 3 together as this is the exact use case for that feature.

I'm adding a commit using that solution. It makes the xdg-compliant directory the default if ~/.fzf.bash is not found. I'm not sure how you feel about that, so I'd like comments. By the time the use runs this install script the ~/.config directory likely exists, so checking for its presence won't yield useful information. Forcing users to set the XDG_CONFIG_HOME variable, at least during installation, is an option, but wouldn't fully follow the spec.

junegunn · 2018-05-28T07:40:58Z

The test script I posted above doesn't pass with the latest commit.

junegunn · 2018-05-28T07:43:17Z

To be precise, I'm seeing the following error messages during the process:

Generate "${XDG_CONFIG_HOME:-~/.config}"/fzf/fzf.bash ... ./install: line 267: bar/fzf/fzf.bash: No such file or directory
OK
Generate "${XDG_CONFIG_HOME:-~/.config}"/fzf/fzf.zsh ... ./install: line 267: bar/fzf/fzf.zsh: No such file or directory
OK
Update fish_user_paths ... Failed

junegunn · 2018-05-28T07:51:03Z

Hmm, I'm thinking again if changing the default directory on fresh installation is a good call, because there are many users who have [ -f ~/.fzf.bash ] && source ~/.fzf.bash committed to their dotfiles repositories. See:

https://github.com/search?q=%5B+-f+~%2F.fzf.bash+%5D+%26%26+source+~%2F.fzf.bash&type=Code

They will expect fzf installer to create the file on that particular path.

How about we add --xdg flag for explicitly selecting $XDG_CONFIG_HOME over the default?

junegunn · 2018-05-29T04:46:28Z

install

+      eval 'config_dir="$config_prefix"'
+      eval "config_dir=$config_dir"
+      eval "config_dir=$config_dir"
+      echo $config_dir


Echo for debugging?

junegunn · 2018-05-29T04:46:44Z

uninstall

+      eval 'config_dir="$config_prefix"'
+      eval "config_dir=$config_dir"
+      eval "config_dir=$config_dir"
+      echo $config_dir


eval doesn't work well with XDG_CONFIG_HOME with whitespaces and it can be a security hole as bash will inadvertently execute any command injected in the variable. FOO="b ls" eval "a=$FOO" Also, replace tilde with $HOME to simplify the expansion.

junegunn · 2018-05-31T03:20:16Z

Pushed a fix that removes evals. See the commit message for the details.

Also I had to change ${XDG_CONFIG_HOME:-~/.config} to ${XDG_CONFIG_HOME:-$HOME/.config}, as the tilde doesn't expand inside double quotes.

mkdir -p ~/.config
unset XDG_CONFIG_HOME
[ -d "${XDG_CONFIG_HOME:-~/.config}" ] && echo found || echo not found
  # not found
[ -d ${XDG_CONFIG_HOME:-~/.config} ] && echo found || echo not found
  # found
[ -d "${XDG_CONFIG_HOME:-$HOME/.config}" ] && echo found || echo not found
  # found

Please review the change and let me know if there's anything else you want to address. I'll merge this if everything's okay.

mark64 · 2018-05-31T04:47:17Z

That looks good. Works for me

junegunn · 2018-06-01T02:55:07Z

Merged, thanks!

ANGkeith · 2019-12-28T12:10:45Z

Hi, I am still new to this XDG Base Directory thing.

Just curious, why is the fzf installed into the XDG_CONFIG_HOME directory instead of ~/.local/lib directory ?

dlagg · 2020-07-05T14:19:26Z

Here another newbie. I have set XDG_CONFIG_HOME to .config , and created, .config/fzf. Should it store dotfiles there without setting any env var? I have to reinstall fzf?

Futarimiti · 2023-07-12T11:27:47Z

Another newbie here with the same wonder. Has someone come up to solve it eventually?

Edit: never mind, just found it here:

The shell init files will be installed to XDG_CONFIG_HOME/fzf if the installation script is called with --xdg for example /usr/local/opt/fzf/install --xdg.

lucaspar · 2024-07-12T17:38:00Z

@junegunn any chance we can have the --xdg to be the default behavior when XDG_CONFIG_HOME is set?

I could help with making this change.

mark64 added 2 commits May 6, 2018 00:06

support for XDG Base Directory Specification

b78f1d1

fixed unset variable error

18dc062

junegunn requested changes May 6, 2018

View reviewed changes

indentation + revert default directory

9f86a16

junegunn requested changes May 8, 2018

View reviewed changes

added trailing / to fish_dir, renamed config_base

e8da781

junegunn added the enhancement label May 13, 2018

junegunn added this to the 0.17.4 milestone May 13, 2018

junegunn assigned mark64 May 13, 2018

applied Junegunn's patch to prevent expanding homedir

0f6aa06

fixed issue with shell filename expansion

307e3c6

handle edge case of malformed XDG_CONFIG_HOME

25916a4

actually handle spaces in XDG_CONFIG_HOME, fix uninstall

a37c848

mark64 and others added 3 commits May 19, 2018 22:56

fixed handling of spaces in fish_dir

b3699ab

[uninstall] Wait for a linefeed when asking for confirmation

fdaae70

Related junegunn#1035

[install/uninstall] Allow XDG_CONFIG_HOME with whitespaces

50e160f

[ -f "$XDG_CONFIG_HOME"/fzf/fzf.bash ] && source "$XDG_CONFIG_HOME"/fzf/fzf.bash

[uninstall] config_prefix should be quoted

335c3ad

combine XDG_CONFIG_HOME and ~/.config path choices

aa7fb93

added --xdg option to install and uninstall

538d11b

junegunn reviewed May 29, 2018

View reviewed changes

mark64 and others added 2 commits May 29, 2018 18:31

no echos

86d2451

[install/uninstall] Avoid using eval

4fe03fd

eval doesn't work well with XDG_CONFIG_HOME with whitespaces and it can be a security hole as bash will inadvertently execute any command injected in the variable. FOO="b ls" eval "a=$FOO" Also, replace tilde with $HOME to simplify the expansion.

[install/uninstall] Code cleanup

6c275d4

junegunn force-pushed the master branch from babfc4c to 6c275d4 Compare May 31, 2018 06:45

junegunn added 2 commits May 31, 2018 19:08

Capitalize XDG

8ecb2f2

Update --help

4793515

junegunn approved these changes Jun 1, 2018

View reviewed changes

junegunn merged commit 2ff1908 into junegunn:master Jun 1, 2018

edubxb mentioned this pull request Nov 11, 2018

Support XDG in fzf plugin Bash-it/bash-it#1268

Merged

rene-descartes2021 mentioned this pull request May 9, 2022

XDG spec compliance, fzf now XDG path liuchengxu/space-vim#486

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for XDG Base Directory Specification #1282

Support for XDG Base Directory Specification #1282

mark64 commented May 6, 2018

junegunn May 6, 2018

mark64 May 6, 2018

junegunn May 8, 2018

junegunn May 6, 2018

junegunn May 6, 2018

junegunn May 6, 2018

mark64 May 6, 2018

junegunn May 8, 2018

junegunn May 8, 2018 •

edited

Loading

mark64 May 12, 2018

junegunn commented May 13, 2018

mark64 commented May 14, 2018

junegunn commented May 14, 2018

junegunn commented May 18, 2018

mark64 commented May 19, 2018

junegunn commented May 20, 2018

junegunn commented May 20, 2018

junegunn commented May 28, 2018 •

edited

Loading

junegunn commented May 28, 2018 •

edited

Loading

mark64 commented May 28, 2018

junegunn commented May 28, 2018

junegunn commented May 28, 2018 •

edited

Loading

junegunn commented May 28, 2018

junegunn May 29, 2018

junegunn May 29, 2018

junegunn commented May 31, 2018

mark64 commented May 31, 2018

junegunn commented Jun 1, 2018

ANGkeith commented Dec 28, 2019 •

edited

Loading

dlagg commented Jul 5, 2020

Futarimiti commented Jul 12, 2023 •

edited

Loading

lucaspar commented Jul 12, 2024

Support for XDG Base Directory Specification #1282

Support for XDG Base Directory Specification #1282

Conversation

mark64 commented May 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junegunn May 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junegunn commented May 13, 2018

mark64 commented May 14, 2018

junegunn commented May 14, 2018

junegunn commented May 18, 2018

mark64 commented May 19, 2018

junegunn commented May 20, 2018

junegunn commented May 20, 2018

junegunn commented May 28, 2018 • edited Loading

junegunn commented May 28, 2018 • edited Loading

mark64 commented May 28, 2018

junegunn commented May 28, 2018

junegunn commented May 28, 2018 • edited Loading

junegunn commented May 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junegunn commented May 31, 2018

mark64 commented May 31, 2018

junegunn commented Jun 1, 2018

ANGkeith commented Dec 28, 2019 • edited Loading

dlagg commented Jul 5, 2020

Futarimiti commented Jul 12, 2023 • edited Loading

lucaspar commented Jul 12, 2024

junegunn May 8, 2018 •

edited

Loading

junegunn commented May 28, 2018 •

edited

Loading

junegunn commented May 28, 2018 •

edited

Loading

junegunn commented May 28, 2018 •

edited

Loading

ANGkeith commented Dec 28, 2019 •

edited

Loading

Futarimiti commented Jul 12, 2023 •

edited

Loading