From: Yuan Fu <casouri@gmail.com>
To: Theodor Thornhill <theo@thornhill.no>
Cc: Eli Zaretskii <eliz@gnu.org>, 61558@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 [thread overview]
Message-ID: <F19064EA-AA19-46AE-AF14-BF829ED06C07@gmail.com> (raw)
In-Reply-To: <5EE12293-A537-4D8B-A17B-A5BCAC36D14A@thornhill.no>
> On Feb 24, 2023, at 10:37 PM, Theodor Thornhill <theo@thornhill.no> wrote:
>
>
>
> On 25 February 2023 05:30:02 CET, Yuan Fu <casouri@gmail.com> wrote:
>>
>> Theodor Thornhill <theo@thornhill.no> writes:
>>
>>> 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!
>>
>>
>> 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
next prev parent reply other threads:[~2023-02-26 8:49 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-16 20:48 bug#61558: 29.0.60; Indentation with c-ts-mode doesn't work in code guarded by #ifdef..#endif Eli Zaretskii
2023-02-17 19:23 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-17 19:32 ` Eli Zaretskii
2023-02-17 19:49 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-18 9:53 ` Eli Zaretskii
2023-02-18 21:11 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-18 21:30 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 7:42 ` Eli Zaretskii
2023-02-19 9:20 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 7:43 ` Eli Zaretskii
2023-02-19 8:01 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-25 4:30 ` Yuan Fu
2023-02-25 6:37 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-26 8:49 ` Yuan Fu [this message]
2023-02-26 10:30 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=F19064EA-AA19-46AE-AF14-BF829ED06C07@gmail.com \
--to=casouri@gmail.com \
--cc=61558@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=theo@thornhill.no \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.