GNU bug report logs - #73404
30.0.50; [forward/kill/etc]-sexp commands do not behave as expected in tree-sitter modes

Previous Next

Package: emacs;

Reported by: Mickey Petersen <mickey <at> masteringemacs.org>

Date: Sat, 21 Sep 2024 05:13:01 UTC

Severity: normal

Merged with 74366

Found in version 30.0.50

Fixed in version 31.0.50

Done: Juri Linkov <juri <at> linkov.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: mickey <at> masteringemacs.org, casouri <at> gmail.com, theo <at> thornhill.no, 73404 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: bug#73404: 30.0.50; [forward/kill/etc]-sexp commands do not behave as expected in tree-sitter modes
Date: Sun, 05 Jan 2025 21:54:12 +0200
> From: Juri Linkov <juri <at> linkov.net>
> Cc: casouri <at> gmail.com,  theo <at> thornhill.no,  mickey <at> masteringemacs.org,
>   73404 <at> debbugs.gnu.org,  monnier <at> iro.umontreal.ca
> 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.




This bug report was last modified 131 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.