~The using double quotes to quote command line option values with spaces (e.g. paths) is allowed on all platforms. Options in config files are handled in the same way now.~ The config file values can be surrounded by quotation marks. This allows for explicit using of whitespaces (e.g. in paths).
utils and libraries: Allow values quoting in config files #14370
pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:20181002-config-quotes changing 1 files +3 −0-
hebasto commented at 4:41 PM on October 2, 2018: member
-
ken2812221 commented at 4:48 PM on October 2, 2018: contributor
Doesn't the path with spaces in it work without quotes? How to specify a path that has quotes at the end?
-
hebasto commented at 5:01 PM on October 2, 2018: member
Doesn't the path with spaces in it work without quotes?
Yes, it works. But it is inconsistent. Compare:
bitcoin-qt -blocksdir="/some/path/with space"with a line in a config fileblocksdir=/some/path/with space - hebasto renamed this:
Allow values quoting in config files
utils and libraries: Allow values quoting in config files
on Oct 2, 2018 -
hebasto commented at 8:32 PM on October 2, 2018: member
How to specify a path that has quotes at the end?
I didn't catch it. Can you provide a use case?
-
practicalswift commented at 8:45 PM on October 2, 2018: contributor
Concept ACK
Seems like a nice usability improvement
- fanquake added the label Utils/log/libs on Oct 2, 2018
-
MarcoFalke commented at 4:40 AM on October 3, 2018: member
This is a breaking change with previous versions? In any case, this needs extensive release notes documentation or update the documentation of our config file parser. (Not sure if we have that user documentation already)
- MarcoFalke added the label Needs release notes on Oct 3, 2018
-
ken2812221 commented at 4:52 PM on October 6, 2018: contributor
If someone is using a weird path
datadir=/path/contains/quote", It would strip out the". It would also strip it out if it only appear once. Also, in command line you can specify-datadir=/path/"contain whitespace", it is still inconsistent with conf file. -
hebasto commented at 5:46 PM on October 7, 2018: member
@MarcoFalke Thank you for your review.
This is a breaking change with previous versions? In any case, this needs extensive release notes documentation or update the documentation of our config file parser. (Not sure if we have that user documentation already)
#14427 addressed to your concerns about user documentation. Would you mind to review it?
-
hebasto commented at 6:33 PM on October 7, 2018: member
@ken2812221 Thank you for your review.
Also, in command line you can specify
-datadir=/path/"contain whitespace", it is still inconsistent with conf file.My bad. I'm going to fix it.
- hebasto renamed this:
utils and libraries: Allow values quoting in config files
utils and libraries: [WIP] Allow values quoting in config files
on Oct 7, 2018 - hebasto renamed this:
utils and libraries: [WIP] Allow values quoting in config files
utils and libraries: Allow values quoting in config files
on Oct 7, 2018 - hebasto force-pushed on Oct 7, 2018
-
hebasto commented at 9:10 PM on October 7, 2018: member
@ken2812221 Fixed. Please re-review.
-
ryanofsky commented at 2:06 AM on October 8, 2018: member
I think it's better for config file parser to either:
- not interpret quotes at all
- interpret quotes in a very simple way like https://en.wikipedia.org/wiki/INI_file#Quoted_values
- interpret quotes in a documented way like https://github.com/toml-lang/toml#user-content-string
than to do what this PR does and try to make config file quoting vaguely resemble posix shell quoting and windows msvcrt command line quoting without actually implementing either scheme.
-
7bc6de6af5
Allow values quoting in config files
The config file values can be surrounded by quotation marks. This allows for explicit using of whitespaces (e.g. in paths).
- hebasto force-pushed on Oct 8, 2018
-
hebasto commented at 7:30 AM on October 8, 2018: member
@ryanofsky Thank you for your review.
interpret quotes in a very simple way like https://en.wikipedia.org/wiki/INI_file#Quoted_values
Agree. Done. Please re-review.
-
ken2812221 commented at 7:41 AM on October 8, 2018: contributor
Concept ACK. This is much better. Also the use can specify any characters that it wants.
-
MarcoFalke commented at 7:51 AM on October 8, 2018: member
This needs tests before merge.
-
hebasto commented at 7:13 PM on October 13, 2018: member
This needs tests before merge.
I've modified the src/test/util_tests.cpp unit tests. Should this patch be placed in this PR?
-
laanwj commented at 5:31 AM on October 16, 2018: member
i'm not sure this is worth it
- you've always been able to use spaces in
.confarguments, this just needs to be documented better - as far as I know, explicit quoting is not part of
.inifile format as used in other software, and that's what our conf format is based on (but let me know otherwise if this is wrong!) - as @MarcoFalke mentioned by this is a breaking change, what if you want to have " around a value? you'll now end up with two sets?
- you've always been able to use spaces in
-
meshcollider commented at 5:35 AM on October 16, 2018: contributor
as @MarcoFalke mentioned by this is a breaking change, what if you want to have " around a value? you'll now end up with two sets?
Especially in, for example, an
rpcpasswordor something -
laanwj commented at 5:37 AM on October 16, 2018: member
yes, there is plenty of tooling/scripting that assumes you can just dump a value after
=, this will all have to be rewritten to take this into account(edit: although arguably, #13143 already complicates this, and provides a use-case for quoting maybe?!)
- laanwj closed this on Oct 18, 2018
- hebasto deleted the branch on Oct 18, 2018
- laanwj referenced this in commit 6746a89519 on Oct 20, 2018
- fanquake removed the label Needs release note on Mar 20, 2019
- PastaPastaPasta referenced this in commit 7644b0fa8e on Jun 27, 2021
- PastaPastaPasta referenced this in commit 1454380ea9 on Jun 28, 2021
- PastaPastaPasta referenced this in commit 931802b239 on Jun 29, 2021
- PastaPastaPasta referenced this in commit 8a9b3ff5f1 on Jul 1, 2021
- PastaPastaPasta referenced this in commit 868f071b9f on Jul 1, 2021
- PastaPastaPasta referenced this in commit 30cdf9c6f7 on Jul 1, 2021
- PastaPastaPasta referenced this in commit 787113a778 on Jul 3, 2021
- DrahtBot locked this on Dec 16, 2021