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

Option with spaces breaks in stowrc #15

Open
cledoux opened this issue May 18, 2016 · 8 comments
Open

Option with spaces breaks in stowrc #15

cledoux opened this issue May 18, 2016 · 8 comments

Comments

@cledoux
Copy link

cledoux commented May 18, 2016

The function that parses .stowrc files, get_config_file_options, splits .stowrc file on spaces. Thus, if a file path or regex has a space, the option breaks because get_config_file_options treats it as two options instead of one. In order to get the correct behavior, a more complicated split function that respects escapes would need to be implemented.

@aspiers
Copy link
Owner

aspiers commented Jun 12, 2016

You're correct. For reference, the guilty line is this one.

I suspect it would be easy to fix this using Text::ParseWords::parse_line or similar.

@aspiers
Copy link
Owner

aspiers commented Jun 12, 2016

Actually, a simpler solution would be to specify that .stowrc must contain exactly one option per line. The manual does not document it supporting multiple options per line, so technically it would not be a regression to change this behaviour (and hopefully not too many people would have more than one option per line).

This would avoid a dependency on a new module for just one obscure corner case.

What do you think?

@cledoux
Copy link
Author

cledoux commented Jun 13, 2016

How big of a deal is adding a dependency to a core module in perl? My background is in python with "batteries included." I'd use a python standard library without hesitation, but I'm afraid I don't know if the same applies to the perl core modules.

If the change to assuming one option per line is made, how can this break an existing installation with multiple options per line? I think the --ignore, --defer, and --override options will fail silently, which might be an issue. Best I can tell, the other options should fail gracefully and output a helpful error message.

That said, the examples in the stow manual do only list one option per line, implying that assuming this behavior is not unreasonable.

@aspiers
Copy link
Owner

aspiers commented Jun 13, 2016

Well it's not a huge deal, but given that I expect most users don't even use ~/.stowrc let alone worry about the corner case of options with spaces, I'd prefer to avoid making the whole user base install another module just for that one small corner case, if there's a simpler alternative.

Thanks for your useful feedback on my idea. Yes, we could certainly make it watch for multiple options per line and fail gracefully. I can't imagine anyone would want to use an option value which contains a string like --ignore ;-)

@aspiers
Copy link
Owner

aspiers commented Aug 23, 2016

@cledoux Sorry, I just realised that I misunderstood/misread your previous point, because I thought Text::ParseWords wasn't a core module. (Back in my regular Perl days, I'm pretty sure it was a CPAN module.) Given that it's now core, you're right that adding a dependency on it is a no-brainer.

That said, I wonder if we should, as the simplicity of the one option per line approach sidesteps the need for more complicated quoting rules. OTOH, http://unix.stackexchange.com/questions/304830/stow-doesnt-use-the-ignore-option-given-in-the-rc-file highlighted that it's unintuitive that using = as the separator in .stowrc (e.g. --ignore=foo) doesn't work, and also that quotes don't get stripped, and it seems that Text::ParseWords offers an easy way to solve this in a single go:

$ perl -MText::ParseWords -de1

Loading DB routines from perl5db.pl version 1.44
Editor support available.

Enter h or 'h h' for help, or 'man perldebug' for more help.

main::(-e:1):   1
  DB<1> print "<$_> " for quotewords("[= \n]", 0, qq{--ignore "foo bar" --exclude=foo\ bar\nbaz qux})
<--ignore> <foo bar> <--exclude> <foo> <bar> <baz> <qux> 

@getawaywithrmdir
Copy link

This was driving me crazy, but I landed on a workaround. I am posting it here in case anybody else is struggling with this.

It's...awkward. If you have a better way, please share!

FYI, I'm on MacOS using zsh. I am using stow to create symlinks for configuration files kept under "~/Library/Application Support/Alfred". I needed to make that bloody space work in the directory name in a local .stowrc file for specifying -d and -t options different from my $HOME .stowrc file. Using quotes, backslash, etc. all failed to work since stow puts everything in single quotes, which (in zsh at least--I'm not familiar with other shells) makes everything literal without a $ parameter in front. Even octal character notation failed, so...

  1. I added export INSERT_SPACE=" " to my .zshrc file to create a permanent environment variable that's just a space. Gross.
  2. I added this to the .stowrc file in the stow directory holding my Alfred files:
-t ${HOME}/Library/Application${INSERT_SPACE}Support/Alfred
-d ${HOME}/dotfiles/Alfred

And now it just works (though you might need to adapt it for whatever shell you're using). Obviously it would be nicer for stow to just not freak out over spaces in directory names, but here we are almost eight years since this issue was opened, so...hope this helps somebody out there!

@aspiers
Copy link
Owner

aspiers commented Apr 16, 2024

I'll gladly accept a PR. This really should be an easy change to implement.

@aspiers
Copy link
Owner

aspiers commented Apr 16, 2024

To be clear, based on my last comment, I think using Text::ParseWords is the way to go not just because it retains backwards-compatibility, but because it also achieves correct parsing for each option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants