Closed neogeographica closed 3 years ago
Other notes:
I guess the match should also only terminate on a single close-brace? We're getting into really unlikely-to-be-used corner cases here, but, yeah. Doable I think with a non-greedy regex. If I want to continue allowing single-character placeholder names... still doable and not too bad.
The error message for bad placeholder format should mention how to do literal braces.
When creating/updating a cmdline, I should do a test format (leaving the None values in) just to make sure that's happy. Fail the create/update if that test fails.
Hmm, had a giant update here but now it's gone. Maybe I had only done preview. :-(
Anyway, there are additional problems with using RE to pull out the placeholder tokens. Like properly handling 3 or more identical braces in a row... what happens if e.g. you want the value for placeholder foo to appear inside literal braces, you would want to something like this to work: {{{foo}}}
So let's not use RE for this. Just walk the string looking for braces. Something like:
token = ""
cmdline_format = ""
prev_undoubled_brace = None
for c in cmdline:
c_is_brace = (c == '{' or c == '}')
if not token:
if prev_undoubled_brace == '{' and not c_is_brace:
token = c
else:
cmdline_format += c
else:
if c == '}' and prev_undoubled_brace != '}':
cmdline_format += handle_placeholder(token)
cmdline_format += c
token = ""
else:
token += c
if c == prev_undoubled_brace:
prev_undoubled_brace = None
elif c_is_brace:
prev_undoubled_brace = c
Also I think even the "value" parts for some placeholder formats should use double-brace if they mean the value to include a brace. This means that handle_placeholder will have to reduce every pair of braces to a single brace when storing the value. (Should setting a value with the "vals" or "run" operation have the same behavior? ... let's say no.) update_cmdline would need to do the reverse, i.e. explode single brace into 2 braces.
For the output of Python string format, to get a literal brace you must double it, i.e. use {{ or }} to get { or } output.
I need to make sure that a doubled brace isn't interpreted as beginning a placeholder token. I think the necessary changes would be:
PLACEHOLDER_RE should be
([^{]|^){([^}{][^}]*)}
... i.e. before the bracket-of-interest, we must have either a non-bracket character or the beginning of the line.PLACEHOLDER_DEFAULT_RE and PLACEHOLDER_TOGGLE_RE now should lose their outer brackets. Similar to ALPHANUM_RE they are just for matching against whatever is inside the brackets. Maybe rename them?
update_values_from_args doesn't need to define
token
; the placeholder REs can now be matched againsta
.handle_placeholder in update_cmdline/handle_cmd_set should run PLACEHOLDER_DEFAULT_RE and PLACEHOLDER_TOGGLE_RE against match.group(2) (instead of match.group(0)), and prepend match.group(1) to any returned result that is being constructed from brackets and key.