Indent version 2.2.12; cuddle-else does not work as described

[David L. Paktor]

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: address@hidden <mailto:address@hidden>

Mobil: 408 761 3220

CuddleElseBugFix.tar.gz
Description: application/gzip