Package: emacs;
Reported by: Eli Zaretskii <eliz <at> gnu.org>
Date: Thu, 16 Feb 2023 20:49:01 UTC
Severity: normal
Found in version 29.0.60
View this message in rfc822 format
From: Yuan Fu <casouri <at> gmail.com> To: Theodor Thornhill <theo <at> thornhill.no> Cc: Eli Zaretskii <eliz <at> gnu.org>, 61558 <at> debbugs.gnu.org Subject: bug#61558: 29.0.60; Indentation with c-ts-mode doesn't work in code guarded by #ifdef..#endif Date: Sun, 26 Feb 2023 00:49:33 -0800
> On Feb 24, 2023, at 10:37 PM, Theodor Thornhill <theo <at> thornhill.no> wrote:
>
>
>
> On 25 February 2023 05:30:02 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>>
>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>
>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>
>>>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>>>> Cc: 61558 <at> 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!
>>
>>
>> Sorry for joining late, I just added some change to support "indent
>> according to previous sibling" requested by someone on emacs-devel, and
>> noticed this change. IIUC, the goal is to indent whatever inside a
>> preproc directive as if the directive doesn’t exist, right? If that is
>> true, we should be fine by just using c-ts-common-statement-offset. Am I
>> missing something?
>>
>> Statements inside labels are indented similarly, and this is the rule
>> used for them:
>>
>> ((parent-is "labeled_statement") point-min c-ts-common-statement-offset)
>>
>> I tried to rewrite the current rule for preproc in the similar fasion,
>> ie, from
>>
>> ((n-p-gp nil "preproc" "translation_unit") point-min 0)
>> ((n-p-gp nil "\n" "preproc") great-grand-parent c-ts-mode--preproc-offset)
>> ((parent-is "preproc") grand-parent c-ts-mode-indent-offset)
>>
>> to
>>
>> ((n-p-gp nil "\n" "preproc") point-min c-ts-common-statement-offset)
>> ((parent-is "preproc") point-min c-ts-common-statement-offset)
>>
>> and the test can pass.
>>
>> Yuan
>
>
> I have no strong opinions on this, but imo that function is pretty heavy at this point, and the rules i supplied are simple enough. C-ts-common-strtement-offset is an engine of its own and pretty complex by now, don't you think?
>
Sure. As long you are satisfied, I’m fine with it. c-ts-common-statement-offset is indeed too heavy. I’m working to improve c-ts-common-statement-offset and make it more like parent-bol (so it’s lighter).
Yuan
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.