Subject: Re: bug#73404: 30.0.50; [forward/kill/etc]-sexp commands do not behave as expected in tree-sitter modes

From: Eli Zaretskii <[email-masked]>
Date: Sun, 05 Jan 2025 21:54:12 +0200

> From: Juri Linkov <[email-masked]>
> Cc: [email-masked],  [email-masked],  [email-masked],
>   [email-masked],  [email-masked]
> Date: Sun, 05 Jan 2025 20:05:53 +0200
> 
> >> The command is 'forward-list'.  With 'list' it will do in ts-modes
> >> the same that 'forward-list' already does in non-ts modes.
> >
> > Thanks.
> >
> > I tried this now in c-ts-mode, and it seems to behave strangely in
> > some cases.
> >
> > First, AFAICT it supports "lists" enclosed in "(..)", "{..}" and
> > "<..>", but not "[..]".  Is that expected?  Why don't we support
> > "[..]"?
> 
> This looks like a bug.  It would be nice if you posted an example.

That's easy:

  C-f C-f src/dispnew.c RET
  M-x -c-ts-mode RET
  C-s [ RET

This brings you to this function:

  static void
  check_rows (struct frame *f)
  {
    for (int y = 0; y < f->desired_matrix->nrows; ++y)
      if (MATRIX_ROW_ENABLED_P (f->desired_matrix, y))
	{
	  struct glyph_row *row = MATRIX_ROW (f->desired_matrix, y);
	  for (int x = 0; x < row->used[TEXT_AREA]; ++x)
	    eassert (row->glyphs[TEXT_AREA][x].frame != 0);
	}
  }

Move point to any of the [ brackets there and type C-M-n -- you will
be moved to the last ) of the function.

Another example on line 152 in dispnew.c:

  struct redisplay_history
  {
    char trace[512 + 100];
  };

Move point to the [ before 512: C-M-n will signal an error.

Any bracket in the file will trigger either the first or the second
behavior.

> > Next, it many times signals an error "No next group", which is less
> > useful than it could be.  For example:
> >
> > w32_get_internal_run_time (void)
> > {
> >   if (get_process_times_fn)
> >     {
> >       FILETIME create, exit, kernel, user;
> >       HANDLE proc = GetCurrentProcess ();
> >       if ((*get_process_times_fn) (proc, &create, &exit, &kernel, &user))
> >         {
> > 	  return ltime (total.QuadPart);
> >         }                              ^
> >     }
> >
> >   return Fcurrent_time ();
> > }
> >
> > If you put point on the semi-colon marked by "^", C-M-n signals an
> > error.  Can't we instead move to the next "list" after
> > "Fcurrent_time"?
> 
> 'C-M-n' doesn't move to the next statement in c-mode,
> since we need to support backward-compatibility.  The key
> that moves to the next statement ignoring nested lists
> in C modes is 'M-e'.

Well, then maybe we could have the more useful behavior as an opt-in
option?

> > Also, it sometimes "misses" what looks like a "list".  For example:
> >
> > s_pfn_Open_Process_Token = (OpenProcessToken_Proc)
> > 			    get_proc_addr (hm_advapi32, "OpenProcessToken");
> >
> > If you put point at the beginning of the line, C-M-n moves to the end
> > of the 2nd parenthesized group, not to the end of the first group.
> 
> Welcome to the twisty maze of tree-sitter grammars.  C-M-n is unable
> to move to the first group because it's inside the node in the AST:
> 
>     (cast_expression "("
>      type: (type_descriptor type: (type_identifier))
>      ")"
>      value: 
>       (call_expression function: (identifier)
>        arguments: 
>         (argument_list "(" (identifier) ,
>          (string_literal " (string_content) ")
>          ")")))
> 
> Both the first and the 2nd parenthesized groups are inside the
> "cast_expression" node.

Sorry, I don't follow: the parentheses of the argument list are also
inside a node, and yet we do recognize them.  What's the difference?

> > As for terminology: the Emacs user manual calls these "groupings
> > delimited by parentheses (or whatever else serves as delimiters in the
> > language you are working with)", or in short "parenthetical group".
> > Why cannot we keep this terminology?  It sounds like it describes its
> > subject as accurate as we can come up with.
> 
> So you prefer "group" over "list"?

As a shorthand; the better term is "parenthesized group".

> Then 'forward-list' should move over "group"?

I'm not sure we should rename the commands, given the legacy.  But
the doc string should use "group" or "parenthesized group", IMO.