Yasuhito FUTATSUKI wrote on Thu, 14 May 2020 21:44 +00:00:
> Thank you for the review and lesson on my commit message. I intended
> to send this commit message with the patch before commit, but missed
> by mistakes.
>
> I added note about quotation characters which could also cause SyntaxError.
>
> Before replace this log message, I'd like to get a review again.
>
Thanks for your diligence.
> [[[
> entries-dump: Escape string-typed attribute values when serializing
> them as Python string literals.
>
> Before this commit, a filesystem node named "foo\bar" (a single,
> 7-character path component) would cause "e.name = 'foo\bar'" to be
> emitted. The unescaped backslash would manifest as a test failure or
> a SyntaxError, depending on the following characters.
>
*nod* Changing the Unicode quotes to double quotes is fine.
(I generally use Unicode quotes so it's clear which quotes delimit code
from English prose and which quotes are literal parts of the code.
However, that's just me.)
> Also, user names can contain "'" (a single quote character) and/or """
> (a double quote character), which would potentially cause a SyntaxError
> even if we choose ether of them to quote string literals.
Something is not clear to me in this paragraph. I do see that if we had
written «printf("'%s'", value)», SyntaxError's could still have resulted;
however, I don't see how that is a reason not to choose single quotes.
We _could_ still have chosen single quotes if we had escaped backslashes,
single quotes, and newlines in «value» before printing it, couldn't we?
It's not clear to me what this paragraph is trying to explain: whether
it's trying to explain why the code generates a «bytes» object as
opposed to a «str» object, or to explain the choice to escape every byte
(even those that don't _require_ escaping according to Python's bytes
literals syntax), or something else.
Also, s/ether/either/.
> * subversion/tests/cmdline/entries-dump.c
> (print_prefix): New function.
> (str_value):
> - Add argument to specify pool.
> - Print human readable value of "value" as is in comment, then set it
> as str value by using hex escaped bytes literal.
> (entries_dump): Add pool argument to str_value() calls.
> (main):
> - Print "Entry" class definition as prefix before entry_dump() or tree_dump()
> - Style fix on if statement (using blocks).
> (): Add include files for assert() and svn_xml_escape_attr_cstring()
> * subversion/tests/cmdline/svntest/main.py
> (run_entiresdump, run_entriesdump_tree): Move definition of "Entry" class
> into generated code by entries-dump execution.
>
> Found by: svn-windows-ra buildbot
> Review by: danielsh
Looks good.
Thanks!
Daniel
> ]]]
Received on 2020-05-15 03:33:16 CEST