bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

[Filippo Argiolas]

Hi, I've been playing with the new treesitter based outline minor mode
and c-ts-mode.

It seems that `outline-hide-subtree' is only working properly when code
is formatted with either GNU or emacs style.

To test try it with a function where the function declarator is preceded on the
same line by either type or storage class and type. It will fold the
function heading instead of the function body.

Problem seems `c-ts-mode--outline-predicate' only checks for
function_declarator nodes at the beginning of line (like in GNU coding
style) but other coding style will start the function line with either
storage class (e.g. static) or type.

Something like this seems to work a little better:

(defun c-ts-mode--outline-predicate (node)
  "Match outlines on lines with function names."
  (or (and (or (equal (treesit-node-type node) "function_declarator")
               (equal (treesit-node-type node) "storage_class_specifier")
               (equal (treesit-node-type node) "primitive_type")
               (equal (treesit-node-type node) "type_identifier"))
           (equal (treesit-node-type (treesit-node-parent node))
                  "function_definition"))
      ;; DEFUNs in Emacs sources.
      (and c-ts-mode-emacs-sources-support
           (c-ts-mode--emacs-defun-p node))))

But it kind of breaks GNU style by adding two outlines per function
definition. Also it seems a bit ugly to enumerate all the node types by
hand, there probably is a nicer way to detect a line with a
function_declarator not at bol.

Any better idea?

Thanks,
Filippo

bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

[Juri Linkov]

> Hi, I've been playing with the new treesitter based outline minor mode
> and c-ts-mode.
>
> It seems that `outline-hide-subtree' is only working properly when code
> is formatted with either GNU or emacs style.
>
> To test try it with a function where the function declarator is preceded on the
> same line by either type or storage class and type. It will fold the
> function heading instead of the function body.
>
> Problem seems `c-ts-mode--outline-predicate' only checks for
> function_declarator nodes at the beginning of line (like in GNU coding
> style) but other coding style will start the function line with either
> storage class (e.g. static) or type.
>
> Something like this seems to work a little better:
>
> (defun c-ts-mode--outline-predicate (node)
>   "Match outlines on lines with function names."
>   (or (and (or (equal (treesit-node-type node) "function_declarator")
>                (equal (treesit-node-type node) "storage_class_specifier")
>                (equal (treesit-node-type node) "primitive_type")
>                (equal (treesit-node-type node) "type_identifier"))
>            (equal (treesit-node-type (treesit-node-parent node))
>                   "function_definition"))
>       ;; DEFUNs in Emacs sources.
>       (and c-ts-mode-emacs-sources-support
>            (c-ts-mode--emacs-defun-p node))))
>
> But it kind of breaks GNU style by adding two outlines per function
> definition. Also it seems a bit ugly to enumerate all the node types by
> hand, there probably is a nicer way to detect a line with a
> function_declarator not at bol.
>
> Any better idea?

Could you please send a few of short examples of each style.
This would help to make a better decision.
And later these examples could be added to tests.

bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

[Filippo Argiolas]

Juri Linkov <juri@linkov.net> writes:

>> Hi, I've been playing with the new treesitter based outline minor mode
>> and c-ts-mode.
>>
>> It seems that `outline-hide-subtree' is only working properly when code
>> is formatted with either GNU or emacs style.
>>
>> To test try it with a function where the function declarator is preceded on the
>> same line by either type or storage class and type. It will fold the
>> function heading instead of the function body.
>>
>> Problem seems `c-ts-mode--outline-predicate' only checks for
>> function_declarator nodes at the beginning of line (like in GNU coding
>> style) but other coding style will start the function line with either
>> storage class (e.g. static) or type.
>>
>> Something like this seems to work a little better:
>>
>> (defun c-ts-mode--outline-predicate (node)
>>   "Match outlines on lines with function names."
>>   (or (and (or (equal (treesit-node-type node) "function_declarator")
>>                (equal (treesit-node-type node) "storage_class_specifier")
>>                (equal (treesit-node-type node) "primitive_type")
>>                (equal (treesit-node-type node) "type_identifier"))
>>            (equal (treesit-node-type (treesit-node-parent node))
>>                   "function_definition"))
>>       ;; DEFUNs in Emacs sources.
>>       (and c-ts-mode-emacs-sources-support
>>            (c-ts-mode--emacs-defun-p node))))
>>
>> But it kind of breaks GNU style by adding two outlines per function
>> definition. Also it seems a bit ugly to enumerate all the node types by
>> hand, there probably is a nicer way to detect a line with a
>> function_declarator not at bol.
>>
>> Any better idea?
>
> Could you please send a few of short examples of each style.
> This would help to make a better decision.
> And later these examples could be added to tests.

You should be able to see it with this example

int bar(void) {
  return 1;
}

static int
foo (int bar) {
  return 2;
}

int main(void) {
  return 3;
}

If you go to the third function and call `outline-hide-subtree' it
replaces the function name with an ellipsis.

Now if you call `outline-show-all' and try to collapse the second
function (GNU style) it will show

foo (int bar) {...

which looks correct to me.

If you show all again and collapse the first one it will do nothing and
say "Before the first heading". This while still showing the outline
icon in the margin.

At the moment I am experimenting with this solution (DEFUN part omitted
for brevity) but it's probably missing some edge case I am not aware of:

(defun c-ts-mode--outline-predicate (node)
  "Match outlines on lines with function names."
  (when-let* ((decl (treesit-node-child-by-field-name
                     (treesit-node-parent node) "declarator"))
              (node-pos (treesit-node-start node))
              (decl-pos (treesit-node-start decl))
              (eol (save-excursion (goto-char node-pos) (line-end-position))))
    (and (equal (treesit-node-type decl) "function_declarator")
         (<= node-pos decl-pos)
         (< decl-pos eol))))

Idea is to match a function declarator in the same line as `node' no
matter what node is at bol.

Ciao,
Filippo

bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

[Juri Linkov]

> At the moment I am experimenting with this solution (DEFUN part omitted
> for brevity) but it's probably missing some edge case I am not aware of:
>
> (defun c-ts-mode--outline-predicate (node)
>   "Match outlines on lines with function names."
>   (when-let* ((decl (treesit-node-child-by-field-name
>                      (treesit-node-parent node) "declarator"))
>               (node-pos (treesit-node-start node))
>               (decl-pos (treesit-node-start decl))
>               (eol (save-excursion (goto-char node-pos) (line-end-position))))
>     (and (equal (treesit-node-type decl) "function_declarator")
>          (<= node-pos decl-pos)
>          (< decl-pos eol))))
>
> Idea is to match a function declarator in the same line as `node' no
> matter what node is at bol.

Thanks, I confirm your solution is better than the current state.
Hope you could find a common denominator that covers all possible cases.

bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

[Filippo Argiolas]

Juri Linkov <juri@linkov.net> writes:

>> At the moment I am experimenting with this solution (DEFUN part omitted
>> for brevity) but it's probably missing some edge case I am not aware of:
>>
>> (defun c-ts-mode--outline-predicate (node)
>>   "Match outlines on lines with function names."
>>   (when-let* ((decl (treesit-node-child-by-field-name
>>                      (treesit-node-parent node) "declarator"))
>>               (node-pos (treesit-node-start node))
>>               (decl-pos (treesit-node-start decl))
>>               (eol (save-excursion (goto-char node-pos) (line-end-position))))
>>     (and (equal (treesit-node-type decl) "function_declarator")
>>          (<= node-pos decl-pos)
>>          (< decl-pos eol))))
>>
>> Idea is to match a function declarator in the same line as `node' no
>> matter what node is at bol.
>
> Thanks, I confirm your solution is better than the current state.
> Hope you could find a common denominator that covers all possible cases.

Seems to work with the code I tested. Guess it's still not perfect, if a
function declaration is splitted over multiple lines it will be folded
where the function name is and I think that's what we want.

A minor problem is with GNU style declarations type and storage class
specifier will go in to the previous subtree when you collapse
everything.

But it's probably good enough. As far as I can tell the way it works now
it folds from one outline to the next. The treesitter way would be to
fold up to where the toplevel node ends.

Maybe that's the job for a more specialized treesit folding package and
this is already good for outline-minor-mode.

By the way, the complete defun with Emacs source parsing restored is:

(defun c-ts-mode--outline-predicate (node)
  "Match outlines on lines with function names."
  (or (when-let* ((decl (treesit-node-child-by-field-name
                         (treesit-node-parent node) "declarator"))
                  (node-pos (treesit-node-start node))
                  (decl-pos (treesit-node-start decl))
                  (eol (save-excursion (goto-char node-pos) (line-end-position))))
        (and (equal (treesit-node-type decl) "function_declarator")
             (<= node-pos decl-pos)
             (< decl-pos eol)))
      ;; DEFUNs in Emacs sources.
      (and c-ts-mode-emacs-sources-support
           (c-ts-mode--emacs-defun-p node))))


Feel free to improve on this if you decide to install it, I never did
the copyright assignment but I think it's trivial enough to enter
without it.

Thanks!
Filippo

bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

[Juri Linkov]

>> Thanks, I confirm your solution is better than the current state.
>> Hope you could find a common denominator that covers all possible cases.
>
> Seems to work with the code I tested. Guess it's still not perfect, if a
> function declaration is splitted over multiple lines it will be folded
> where the function name is and I think that's what we want.
>
> A minor problem is with GNU style declarations type and storage class
> specifier will go in to the previous subtree when you collapse
> everything.
>
> But it's probably good enough. As far as I can tell the way it works now
> it folds from one outline to the next. The treesitter way would be to
> fold up to where the toplevel node ends.
>
> Maybe that's the job for a more specialized treesit folding package and
> this is already good for outline-minor-mode.

outline-minor-mode is designed to always fold from the current heading line
until the next heading line.  Maybe outline-minor-mode could be extended
to detach the heading from the outline boundaries.  For example,
by adding separate regexps/functions to match outline boundaries
while outline-regexp/outline-search-function will still be used
only for the heading line.

> By the way, the complete defun with Emacs source parsing restored is:
>
> (defun c-ts-mode--outline-predicate (node)
>   "Match outlines on lines with function names."
>   (or (when-let* ((decl (treesit-node-child-by-field-name
>                          (treesit-node-parent node) "declarator"))
>                   (node-pos (treesit-node-start node))
>                   (decl-pos (treesit-node-start decl))
>                   (eol (save-excursion (goto-char node-pos) (line-end-position))))
>         (and (equal (treesit-node-type decl) "function_declarator")
>              (<= node-pos decl-pos)
>              (< decl-pos eol)))
>       ;; DEFUNs in Emacs sources.
>       (and c-ts-mode-emacs-sources-support
>            (c-ts-mode--emacs-defun-p node))))
>
>
> Feel free to improve on this if you decide to install it, I never did
> the copyright assignment but I think it's trivial enough to enter
> without it.

Thanks, will test it more before installing.

bug#74448: 30.0.92; c-ts-mode outlines only work with GNU or emacs style

[Juri Linkov]

close 74448 31.0.50
thanks

> Seems to work with the code I tested. Guess it's still not perfect, if a
> function declaration is splitted over multiple lines it will be folded
> where the function name is and I think that's what we want.
>
> A minor problem is with GNU style declarations type and storage class
> specifier will go in to the previous subtree when you collapse
> everything.

Actually this problem exists not only for declarations,
but also for preprocessor directives.  For example:

  #ifdef HAVE_XDBE
  static void
  x_set_inhibit_double_buffering (struct frame *f,
                                  Lisp_Object new_value,
                                  Lisp_Object old_value)

a dangling #ifdef will inevitably remain in the previous outline.
And I see no clear way to split such code to outlines
with function declaration headings.

> But it's probably good enough. As far as I can tell the way it works now
> it folds from one outline to the next. The treesitter way would be to
> fold up to where the toplevel node ends.

Probably it would be sufficient to define one additional function
'outline-prefix-function' that will find the beginning of either
the current outline or the next outline.
And it can be called after every call of 'outline-search-function'
to adjust the found position, but only for hiding outlines.
Still not sure if it can theoretically handle all cases.

> Maybe that's the job for a more specialized treesit folding package and
> this is already good for outline-minor-mode.

Indeed, maybe simpler would be to write a new package from scratch,
since the design of outline-minor-mode is too line-oriented.

> By the way, the complete defun with Emacs source parsing restored is:
>
> (defun c-ts-mode--outline-predicate (node)
>   "Match outlines on lines with function names."
>   (or (when-let* ((decl (treesit-node-child-by-field-name
>                          (treesit-node-parent node) "declarator"))
>                   (node-pos (treesit-node-start node))
>                   (decl-pos (treesit-node-start decl))
>                   (eol (save-excursion (goto-char node-pos) (line-end-position))))
>         (and (equal (treesit-node-type decl) "function_declarator")
>              (<= node-pos decl-pos)
>              (< decl-pos eol)))
>       ;; DEFUNs in Emacs sources.
>       (and c-ts-mode-emacs-sources-support
>            (c-ts-mode--emacs-defun-p node))))
>
>
> Feel free to improve on this if you decide to install it, I never did
> the copyright assignment but I think it's trivial enough to enter
> without it.

Thanks, this logic makes sense.  So after testing more cases I pushed it
to master.