Jonathan Gilbert wrote:
> At 10:12 PM 3/18/2007 -0500, I wrote:
> [snip]
>> In light of this, I would like to revise my proposal to suggest that the
>> macro be named either EOL or NL (to keep it as short as possible), that it
>> be defined in a Subversion-internal header so that it is not visible to
>> outside code (as it lacks the SVN_ prefix that would otherwise keep it from
>> polluting the namespace), and that the EOL or NL tokens be lined up so that
>> they terminate in the 79th column. In the event that a line of text really
>> cannot be broken up and it is long enough that the EOL or NL token could
>> not be lined up in this manner with at least 2 characters of separation
>>from the text, the EOL or NL should be placed wherever the end of that
>> particular line is with the two spaces of separation.
>
> In the hopes of somehow at least partly extricating my foot from my mouth,
> attached is a revised patch which uses NL instead of SVN_NL, defines this
> new namespace-less macro in include/private/svn_compat.h (following a brief
> chat on IRC with C. Michael Pilato) and lines things up so that, as much as
> possible, they fit nicely into an 80x25 console window. I even reviewed the
> changes by opening the files in VIM on Windows in a default 80x25 console
> window. :-)
>
> I also thought it would make sense to propose the use of NL over
> APR_EOL_STR only for multiline string literals, so I reverted the changes
> in the previous version of the patch which changed APR_EOL_STR where used
> in other places, such as parameters to functions, newline matching, etc.
> The patch is a bit smaller, though not a lot since the bulk of the changes
> still apply and are simply a little bit different.
>
> Thoughts? Ready to be applied?
Given that this only affects just 4 source files, and the odd
namespaceless nature of the new define, I'd personally opt for just
doing "#define NL APR_EOL_STR" in each file that needs it.
Apart from that, +1.
Max.
Received on Wed Mar 28 00:58:36 2007