6 maj 2013 kl. 23.39 skrev Daniel Shahaf:
The patch achieves its end in an unmaintainable way. It's liable to
break when APR's implementation details change, and since it
ultimately
attempts to parse something which wasn't intended to be parsed, it
might
do the _wrong_ thing (as opposed to just not doing anything) someday.
No, APR is constrained by its API; it cannot change arbitrarily.
I can't see today what other circumstances APR would call errfn->("%s:
%s: %c\n", _, "invalid option", _) in, but they don't guarantee us
that
there won't be other circumstances so I tend not to assume that they
won't. That's how I treat Subversion APIs that I write and call, and
that's how I treat APR too: I rely on promises, not on implementation
details.
No, the patch will translate "%s: invalid option: %s\n" and a few
other messages as specified by the .po files. There is no way it can
"do the wrong thing".
The worst that can happen is that someone changes the spelling or
formatting of one of the messages in APR. The consequence is then that
this message will be displayed untranslated, and that the translators
may not immediately get to know about it since the changed strings are
not seen by the gettext string extractor. Clearly, this is no worse
than the present situation.
Yes, this patch relies on promises made by the APR - the errfn
signature and semantics.
Should APR eventually contain a more sophisticated error-reporting
interface in the future, the proposed code could of course be retired
in favour of a use of that interface. Until then, the patch would
serve its purpose.
Since the patch:
1) solves an annoying problem,
Which no one complained about.
Two translators have pointed out the defect. Either should be
sufficient.
Keep in mind that error message in the libraries are translated, and
'svn help' is translated. Is it really that important to translate
'invalid option'? If it is, wouldn't someone have complained about it
on users@?
Surely the worthiness of bugs to be fixed isn't measured in the number
of complaints to the users@ mailing list?
Of course it's important to translate that message -- if localisation
is important at all, that is.
Feel free to point me at another place in svn's code that ignores API
non-promises or parses something that wasn't intended to be parsed.
The suggested patch does not misuse the API. Should APR change,
crashes or corruption cannot occur; the API guarantees that.
