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

Support comments within FZF_DEFAULT_OPTS_FILE content #3961

Closed
5 of 10 tasks
daephx opened this issue Aug 9, 2024 · 5 comments
Closed
5 of 10 tasks

Support comments within FZF_DEFAULT_OPTS_FILE content #3961

daephx opened this issue Aug 9, 2024 · 5 comments
Assignees

Comments

@daephx
Copy link

daephx commented Aug 9, 2024

Checklist

  • I have read through the manual page (man fzf)
  • I have searched through the existing issues
  • For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.54.3 (af4917d)

OS

  • Linux
  • macOS
  • Windows
  • Etc.

Shell

  • bash
  • zsh
  • fish

Problem / Steps to reproduce

When defining options via FZF_DEFAULT_OPTS_FILE the config parser doesn't support comment strings,
resulting in an invalid command line string error.

Example fzf config:

FZF_DEFAULT_OPTS_FILE="~/.config/fzf/config"

# Jump back to the first item when query text changes
--bind='change:first'

# Additional key bindings for feature toggles
--bind='alt-a:toggle-all'
--bind='alt-g:ignore'
--bind='alt-p:toggle-preview'
--bind='alt-s:toggle-sort'
--bind='alt-y:yank'

While the typical use of FZF_DEFAULT_OPTS doesn't easily support comments in this fashion do to how string work within shell scripts, I sort of just assumed this would work in the config file because other tools with a similar configuration format such as ripgrep do support comments within the file's contents?

Example ripgrep config:

# Search hidden files and directories
--hidden

# follow symlinks
--follow

I'm not all that familiar with Golang, but after messing around with it a bit, I managed to add a basic sanitization step before the file content is sent to the options parser.

You can find the PR here: #3960
This seems like an easy enough addition, but if the implementation isn't ideal, I'd love to learn about it, thanks!

@junegunn
Copy link
Owner

See mattn/go-shellwords#58

Then we can

diff --git a/src/options.go b/src/options.go
index 1593d6f..450ce38 100644
--- a/src/options.go
+++ b/src/options.go
@@ -2915,6 +2915,12 @@ func postProcessOptions(opts *Options) error {
 	return processScheme(opts)
 }
 
+func parse(str string) ([]string, error) {
+	parser := shellwords.NewParser()
+	parser.ParseComment = true
+	return parser.Parse(str)
+}
+
 // ParseOptions parses command-line options
 func ParseOptions(useDefaults bool, args []string) (*Options, error) {
 	opts := defaultOptions()
@@ -2928,7 +2934,7 @@ func ParseOptions(useDefaults bool, args []string) (*Options, error) {
 				return nil, errors.New("$FZF_DEFAULT_OPTS_FILE: " + err.Error())
 			}
 
-			words, parseErr := shellwords.Parse(string(bytes))
+			words, parseErr := parse(string(bytes))
 			if parseErr != nil {
 				return nil, errors.New(path + ": " + parseErr.Error())
 			}
@@ -2940,7 +2946,7 @@ func ParseOptions(useDefaults bool, args []string) (*Options, error) {
 		}
 
 		// 2. Options from $FZF_DEFAULT_OPTS string
-		words, parseErr := shellwords.Parse(os.Getenv("FZF_DEFAULT_OPTS"))
+		words, parseErr := parse(os.Getenv("FZF_DEFAULT_OPTS"))
 		if parseErr != nil {
 			return nil, errors.New("$FZF_DEFAULT_OPTS: " + parseErr.Error())
 		}

@daephx
Copy link
Author

daephx commented Aug 12, 2024

Nice! I did play around with it a bit, and it looks like I was going in the right direction but didn't land on anything solid.
Hopefully it gets accepted! 👍

@andelink
Copy link

Hi @junegunn, do you have an idea of when 0.55.0 will be released with these changes?

@junegunn
Copy link
Owner

In a few days, hopefully. I was hoping to get #3951 sorted out, but we're not making much progress, so 0.55.0 will probably be released without it.

@andelink
Copy link

Sounds good. Thanks for the quick response!

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

No branches or pull requests

3 participants