Indent version 2.2.12; cuddle-else does not work as described
"David L. Paktor" <[email protected]> Fri, 20 Mar 2020 10:24:01 -0700
| Newsgroups | gmane.comp.gnu.indent.bugs |
|---|---|
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format.
--------------2CC21460ED269304AFD99396
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Greetings.
I hope there are still people who still support and maintain indent
It is a very useful tool for many things, including managing and
untangling "legacy" code, which is what recently brought it back to my
attention.
I was somewhat frustrated, though, to find that, in version 2.2.12, the
"cuddle-else" option does not work as expected.
Using the options |*-npro -st -ce -bl -bli0 -blf -cdw -i4 -nut -sc -npsl
-ts4*| , it generated:
*| } else|**
****||**|{|*
instead of
*| } else {|*|
|
which is what the man-page promises.
So I directed my attentions to the indent program itself.
The attached file contains my proposed fix, in diff format: Each file
named in the form *foobar.x_diffs* contains the diff between the
released *foobar.x* and my changes. I hope that whoever is supporting
and maintaining this useful tool will incorporate my changes into a new
release, and will do so in short order. Thank you in advance.
I will now explain my changes, below:
There are six categories of changes:
1.
The ones essential to the fix
2.
Bringing some variables out of local scope to global, in order to
facilitate tracing during debug.
This had other ramifications, which needed to be accommodated.
3.
Declaring those variables as "extern"s caused some naming conflict
("shadow"ing) errors.
I made some changes to mitigate the errors, then found found that
they were too numerous to overcome, so I left a large comment for
future reference.
4.
There was a passage of dead code that was not obvious, so I made it
obvious
5.
Changing the version number (You might want to change it again; I
used 2.2.13Beta )
6.
Punctuation, capitalization and coding nit-picks.
------------------------------------------------------------------------
|::::::::::::::||
||src/indent.c_diffs||
||::::::::::::::||
||134a135,138||
||> codes_ty type_code = start_token;||
||> BOOLEAN force_nl = false;||
||> ||
||> ||
|
These are the variables I brought out of local scope to global, to more
easily follow during debug
------------------------------------------------------------------------
|159,160d162||
||< codes_ty * type_code,||
||< BOOLEAN * force_nl,|
These are the variables I brought out of local scope to global, to more
easily follow during debug
------------------------------------------------------------------------
|159,160d162||
||< codes_ty * type_code,||
||< BOOLEAN * force_nl,|
Eliminate pointers to the now-global variables, which had been passed as
parameters to the search_brace() function.
These caused some of the "shadow"ing errors, and (at first...)
interfered with tracing during debug
------------------------------------------------------------------------
|187c189||
||< cur_token = *type_code;||
||---||
||> cur_token = type_code;|
No longer being pointers, we could access the globals directly.
------------------------------------------------------------------------
|198c200,204||
||< /* Ignore buffering if no comment stored. */||
||---||
||> if ( (parser_state_tos->last_token == sp_else )||
||> && settings.cuddle_else )||
||> {||
||> force_nl = false;||
||> }||
||199a206||
||> /* Ignore buffering if no comment stored. */|
This was one essential part of the fix. If we are here, we know the
current token is a left-bracket.
If the previous token was an "else", and cuddle_else is in effect, we
do not want to force the next line out until the left-bracket is
incorporated into the output line.
------------------------------------------------------------------------
|320c327||
||< if (((*type_code == sp_paren) && (*token == 'i') &&
/* "if" statement */||
||---||
||> if (((type_code == sp_paren) && (*token == 'i') && /*
"if" statement */||
||322c329||
||< ((*type_code == sp_else) && /* "else" statement */||
||---||
||> ((type_code == sp_else) && /* "else" statement */||
||327c334||
||< *force_nl = false;||
||---||
||> force_nl = false;||
||331c338||
||< *force_nl = true;||
||---||
||> force_nl = true;||
||346c353||
||< if (*force_nl)||
||---||
||> if (force_nl)||
||348c355||
||< *force_nl = false;||
||---||
||> force_nl = false;||
||384c391||
||< if (*type_code != code_eof)||
||---||
||> if (type_code != code_eof)||
||387c394||
||< *type_code = lexi();||
||---||
||> type_code = lexi();||
||412c419||
||< (*type_code == newline || *type_code == comment) &&||
||---||
||> (type_code == newline || type_code == comment) &&||
||418c425||
||< } else if (*type_code == newline) {||
||---||
||> } else if (type_code == newline) {||
||428c435||
||< if ((*type_code == ident) && *flushed_nl &&||
||---||
||> if ((type_code == ident) && *flushed_nl &&|||
All of the above are in the *search_brace()* function; no longer
pointers, we're accessing the global variables directly.
------------------------------------------------------------------------
|451d457||
||< codes_ty type_code = start_token;||
||462,463d467||
||< BOOLEAN force_nl = false;||
||< |
These are where the now-global variables had been in local scope
------------------------------------------------------------------------
|548c552||
||< if (!search_brace(&type_code, &force_nl, &flushed_nl,
&last_else,||
||---||
||> if (!search_brace(&flushed_nl, &last_else,|
No longer parameters to search_brace()
------------------------------------------------------------------------
|581c585||
||< return file_exit_value; /* RETURN */||
||---||
||> return file_exit_value;
/* RETURN */||
||610c614||
||< * indentation. this is||
||---||
||> * indentation. This is|
The punctuation and capitalization nit-picks
------------------------------------------------------------------------
|::::::::::::::||
||src/handletoken.c_diffs||
||::::::::::::::||
||1121c1121,1123||
||< if ((!parser_state_tos->in_decl && !settings.btype_2) ||||
||---||
||> if ( !((parser_state_tos->last_token == sp_else ) &&||
||> settings.cuddle_else ) &&||
||> ((!parser_state_tos->in_decl && !settings.btype_2) ||||
||1124c1126||
||< !settings.braces_on_func_def_line))||
||---||
||> !settings.braces_on_func_def_line)) )||
|
The other essential part of the fix. Similar explanation...
------------------------------------------------------------------------
|1313a1316||
||> *e_code = '\0';|
The coding nit-pick.
------------------------------------------------------------------------
|1373,1381c1376,1384||
||< else if (!settings.braces_on_struct_decl_line &&||
||< (parser_state_tos->block_init != 1))||
||< {||
||< *force_nl = true;||
||< }||
||< else||
||< {||
||< /* what ? */||
||< }||
||---||
||> // else if (!settings.braces_on_struct_decl_line &&||
||> // (parser_state_tos->block_init != 1))||
||> // {||
||> // *force_nl = true;||
||> // }||
||> // else||
||> // {||
||> // /* what ? */||
||> // }|
This is the dead-code passage, killed by the surrounding #if 0 ... #endif
------------------------------------------------------------------------
|::::::::::::::||
||src/output.h_diffs||
||::::::::::::::||
||55c55||
||< int force_nl,||
||---||
||> BOOLEAN dump_nl,|
This was another place that caused some of the "shadow"ing errors. The
*dump_line()* function is only ever called with a *true* for that
parameter; technically, if could have been eliminated completely, but
that would have required chasing down all the places where it is called.
It was easier to just rename the parameter. (Not that it helped,
ultimately, but it's a start...).
------------------------------------------------------------------------
|::::::::::::::||
||src/output.c_diffs||
||::::::::::::::||
||997c997||
||< int force_nl,||
||---||
||> BOOLEAN dump_nl,||
||1010c1010||
||< else if (force_nl)||
||---||
||> else if (dump_nl)||
||1293c1293||
||< if (buf_break_used && (s_code != e_code) && force_nl)||
||---||
||> if (buf_break_used && (s_code != e_code) && dump_nl)|
Renaming the parameter to mitigate the "shadow"ing errors.
------------------------------------------------------------------------
|::::::::::::::||
||src/indent.h_diffs||
||::::::::::::::||
||350a351,363||
||> /*||
||> * We moved these two from local scope to global to facilitate
tracing||
||> * during debug. We would have liked to declare them
"extern"s, but||
||> * we ran into naming conflict ("shadow"ing) errors, and they
were||
||> * too numerous to overcome at the time. We reluctantly leave
this||
||> * notation while we comment-out the "extern" declarations we
would||
||> * have liked to have here, and hope some future maintainer
will||
||> * diligently fix them. DLP, 20 Mar 2020||
||> */||
||> /* extern codes_ty type_code;||
||> * extern BOOLEAN force_nl ;||
||> */||
||> ||
|
Comment explains it all...
------------------------------------------------------------------------
|::::::::::::::||
||configure_diffs||
||::::::::::::::||
||583,584c583,584||
||< PACKAGE_VERSION='2.2.12'||
||< PACKAGE_STRING='GNU Indent 2.2.12'||
||---||
||> PACKAGE_VERSION='2.2.13Beta'||
||> PACKAGE_STRING='GNU Indent 2.2.13Beta'||
||3063c3063||
||< VERSION='2.2.12'||
||---||
||> VERSION='2.2.13Beta'|
Version number change.
------------------------------------------------------------------------
Again, please incorporate my changes into a new release, and in short
order. Thank you in advance.
Of course, if you have any questions, please reply to this letter.
--
David L. Paktor
Sr Firmware Engineer — Embedded Software
Mail: [email protected] <mailto:[email protected]>
Mobil: 408 761 3220
--------------2CC21460ED269304AFD99396
Content-Type: application/gzip;
name="CuddleElseBugFix.tar.gz"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
filename="CuddleElseBugFix.tar.gz"
H4sICAOmdF4AA0N1ZGRsZUVsc2VCdWdGaXgudGFyAO1ZW2/bNhTOc37FSR7iCxxHFCXZRpsO
3RYUwbq0SNvtZYAgU1QsRJYCiaqTtfnvO6QUSZbkOsnSeMV8kPgiHn7nQvLwI53E7MgPXR6K
IbNd3/OSnScXDcWyDPlORqZWfZdimCN9h1CLmkTXTWu0oxGDWuYOaE/vSlPSRDgxwI7ruHEa
/L1Sb137DyqEGg6h5oDQ8e4rYJHLE1vcQCbi5orb8ln+/RhksoQtoksevkD9n9+9e3vy+ixv
Bi+KGbfDAAp9zwkSLjXlHzEnA2JpLrH03ZdKoWavX1oc5Bo1C/3CxmCXjEeMjCe5ooJL48w3
tNwvoF7sHh4eov02rYoSmYyZrmkDXTMqkFKO+nB6EUYxh2nqeTz2wwvwPQgj9H8+x6WDacFW
dwj9o5otKajahe6VEyc8tjGBgqP55PBV4CTizhHM7JXNMVfQq/VGOTiAhAuBZpMhS1034O2q
X5pdK2NSGYyq3GLgE0fXrNrzBwVNdY1RfVTLmwy82y0HIg8TM8HDnowK2+7C7/gd9SizvO97
+6BSpSytTGu3+yTgVNfR/UnNfSkt7svU9yCHU3jyyVp3M7RHgWFmGaX1WSml3xzddstNPUoJ
go7XgIo4vQ9mpkYNi1GTtsyCArK3YiDLdmqMEcR84mDHBqMTUgFVXpWjsXesapHNI6/uotRc
oUixAtFJfWCqMwYCfu13e22etmkZRGcGaZ2Iy/Mw5IvADzl8/QrLDfnilNN91SRcBbQSxyBj
ZuhtQ3ILqhDVklnC9rAmtXtR6bmqo6GjWWrWBq1myHdzL3F6BGky464c+Ebwquf9OhomcZGS
tO9Qa/dEw9IHhkVdwxq172Df2CVfwq6Js9809VrMewl3Yjazp7HDePeg3CThoNgO8WMRBn5R
m4tM8aAlEzW8VR3NMWHmuD7uMRdpHILny23o2hf2ZyfA1d8Y4W8KVrrzk4+fzs/ai+UjbSyh
WkRjFmmrmmulDxkldoQfhUMQMz8BP1kxkx+K9jFH2zTzyyRB/j9zQuQUagJ/l0PAGv5PcKRq
/N+klrXl/88hhOiEyZcBvtB6rd1rslY/tF3OZKWEvYKVTlVJ0nu4jbSUG9jr3p/+yhLcXGTt
/Leh+ziPMXJD5sCq14qKuqyUiR2FtpeGuEa4Z6ttqo3P3KcbcndCCcWjF7kj3v2CDXT+0jp4
HqEjKo9lODx0ZMlP1VpW7J5txhLkY0yoqJU9maeWMtgyKNMgYpe2H/pCMh2C8ZX9vtR5ToMA
lu23NVdXwmDJXswcAT/Jkl3tn+X16OiR4ZZdHxZu2e9LDaMZbtl+W3N1JcxSuNX+m1r/sv5H
qbhKxXC2ofsfTRuZ9fsfw6Db+v8cYpqsOGz5eNzMpLxkKctbyWPddH6lGjft/Fb+tVTW/6bu
fzWDkMb6p9p2/T+HTCYjhv+NCrC2CFTrANHwrCVfcphit2679ika8+7IhPQJZfLlzgtsnKYe
Hk+5c2mnCXfVfV5S3MBkTEkd3tssPKR/4cSmB2JDkpS//2xq/yeEGs3ff8h2/T+HUFNzMPED
TL/ko325hPpyEf3JYR59xqUjZhwXrFhE4MXRHJAxOwEkLLrChxFcBNEUv+Mnz2F+4EtujfQY
P4cXBVZWLNRPCC6fphdDib6I0sCFmfOZQ+BfSkMRSArvxFzanMM+vxY8DveTAUxTsQS24BA7
oSxXEYTOXAKzKPQCnwno7iczx40W+/i0BzyOoxgRnFBFcoNdY76EJSLESOc8jtJE+oBBxyya
c0Cajj1A+HOuHI55gMcMJxTBDQRc+i0vhpawwii75UGS7wdcupnfox7iHqvQ7oLKQ1XaiVRU
6VgCW06N+jZD57NYZjL/iXTTS0WKKZs7mA385zEs/CBYzj0OzAVXjnv+tcruEB//+vb9AHQN
fscFoGu6pvqoY0kfMjfL68/KD2UKOW8v9oPicPSiQPm/ltQfSuSy8S9wAn2v4r+znv9R3arX
f83Y3v89i5hjOjDHBsvfkYG9f/3Lb6/fnNh/nJx/OH13dtzRh/qQ6J1K04eP56dnb447b84+
wakiD3CnlLGwdgz6MxdOp9K8CidXpJpFmXyRvLDhTs73Wk1sOqtb2cpWtvLfl38A9DJYIAAo
AAA=
--------------2CC21460ED269304AFD99396--