bug#61558: 29.0.60; Indentation with c-ts-mode doesn't work in code guarded by #ifdef..#endif

[Theodor Thornhill]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Theodor Thornhill <theo@thornhill.no>
>> Cc: 61558@debbugs.gnu.org
>> Date: Sat, 18 Feb 2023 22:30:06 +0100
>> 
>> >> Typing RET at the end of the two marked lines goes to column zero
>> >> instead of the expected non-zero column.  So it sounds like #define
>> >> and #undef are also not handled correctly yet.
>> >
>> > Yeah, luckily it indents correctly after you start to type.  I'll try to
>> > see if I can make some specific handling for this.
>> >
>> > Theo
>> >
>> 
>> Scratch that.  Can you test this for me?  I think I got it.
>
> Thanks, this seems to work better.  Some problems still remain,
> though.
>
> Line 807 of dispnew.c:
>
> #if defined (HAVE_WINDOW_SYSTEM) && ! defined (HAVE_EXT_TOOL_BAR)
>   /* Clear the matrix of the tool-bar window, if any.  */
>   if (WINDOWP (f->tool_bar_window))   <<<<<<<<<<<<<<<<<<
>     clear_glyph_matrix (XWINDOW (f->tool_bar_window)->current_matrix);
> #endif
>
> Type RET at the end, then type '{' and RET.  The '{' gets indented
> correctly, but there's no additional two-column indent after RET that
> follows '{'.

This is due to how 'c-ts-common-statement-offset' works, which seems to
assume balanced pairs.  I think this is "unrelated" to this bug.

>
> RET after preprocessor directives outside of functions indents by 2
> columns.  For example, here:
>
> #if 0
> /* Swap glyphs between two glyph rows A and B.  This exchanges glyph
>    contents, i.e. glyph structure contents are exchanged between A and
>    B without changing glyph pointers in A and B.  */
>
> If you type RET after "#if 0", point goes to column 2, not zero.
> Interestingly, if you type RET at the end of the last line of the
> following comment, point goes to column zero, as expected.

This one I'll fix.

>
> Line 1357 of dispnew.c:
>
> static void
> free_glyph_pool (struct glyph_pool *pool)
> {
>   if (pool)
>     {
> #if defined GLYPH_DEBUG && defined ENABLE_CHECKING  <<<<<<<<<<<<<<<
>       /* More freed than allocated?  */
>       --glyph_pool_count;
>       eassert (glyph_pool_count >= 0);
> #endif
>       xfree (pool->glyphs);
>       xfree (pool);
>     }
> }
>
> Type RET at the end of the indicated line: point goes to column 6, as
> expected.  But if you then type "{ RET", point gets indented by 4
> columns, not by 2.  And even typing a semi-colon afterwards doesn't
> fix the indentation.
>
> Similarly here:
>
> static void
> adjust_frame_glyphs_for_window_redisplay (struct frame *f)
> {
>   eassert (FRAME_WINDOW_P (f) && FRAME_LIVE_P (f));
>
>   /* Allocate/reallocate window matrices.  */
>   allocate_matrices_for_window_redisplay (XWINDOW (FRAME_ROOT_WINDOW (f)));
>
> #if defined (HAVE_X_WINDOWS) && ! defined (USE_X_TOOLKIT) && ! defined 
> (USE_GTK)
>   /* Allocate/ reallocate matrices of the dummy window used to display
>      the menu bar under X when no X toolkit support is available.  */
>   {  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>     /* Allocate a dummy window if not already done.  */
>     struct window *w;
>     if (NILP (f->menu_bar_window))
>
> The indicated line is 2166 in dispnew.c.  If you type RET there, point
> will be indented to column 6, not 4 as expected.  And if you type RET
> at the end of the following comment line, not only will point be
> over-indented, but the comment itself will also be reindented
> incorrectly.
>
> Anyway, this works much better than the original code, and I saw no
> regressions, so I think you should install this.  Perhaps consider
> adding comments explaining any tricky parts of handling this, so that
> future hackers will know what to do and what to avoid.  Bonus points
> for adding tests for this, so that we don't easily regress in the
> future.
>

Great! Will do :-)

> Thanks!

No problem!