brownts / gpr-ts-mode

GNAT Project major mode using tree-sitter for Emacs
GNU General Public License v3.0
0 stars 1 forks source link

Indentation points #1

Open simonjwright opened 1 month ago

simonjwright commented 1 month ago

gpr-mode indented like these:

   for Source_Dirs use Common.Paths &
     (
      "adainclude",
      "rp2350_svd",
      FreeRTOS.Source,
      FreeRTOS.Portable_Base & "RISC-V"
     );

   for Switches ("s-traceb.adb") use ALL_ADAFLAGS & ("-g")
      & NO_SIBLING_ADAFLAGS & ("-fno-inline-functions-called-once");

but gpr-ts-mode like these:

   for Source_Dirs use Common.Paths &
                         (
                          "adainclude",
                          "rp2350_svd",
                          FreeRTOS.Source,
                          FreeRTOS.Portable_Base & "RISC-V"
                         );

   for Switches ("s-traceb.adb") use ALL_ADAFLAGS & ("-g")
                                       & NO_SIBLING_ADAFLAGS & ("-fno-inline-functions-called-once");

It looks as though gpr-mode treats for-use statements rather like expressions, whereas gpr-ts-mode indents to the first token after use.

I don’t say there’s anything wrong, just something I’ll need to get used to .. well worth the effort, thank you!

Looking at the second example, does gpr-ts-mode have a line length (like the 79 in ada-mode) that it’ll automatically fold long lines to, or does it require the user to insert newlines where they want the folds?

brownts commented 1 month ago

It looks as though gpr-mode treats for-use statements rather like expressions, whereas gpr-ts-mode indents to the first token after use.

Currently, gpr-ts-mode will try to keep the indentation of an expression together, that's why it aligns that way. I think I was trying to be clever, because it seemed to make sense to me that you'd want to keep the expression together, but the more I have thought about it, I wondered if that was really what people were expecting. I'd like the indentation to do what people think it should do. If you put the first part of the expression on the next line, it will align to that.

   for Source_Dirs use
     Common.Paths &
       (
        "adainclude",
        "rp2350_svd",
        FreeRTOS.Source,
        FreeRTOS.Portable_Base & "RISC-V"
       );

   for Switches ("s-traceb.adb") use
     ALL_ADAFLAGS & ("-g")
       & NO_SIBLING_ADAFLAGS & ("-fno-inline-functions-called-once");

I also have an additional indentation parameter (above what gpr-mode has), gpr-ts-mode-indent-exp-item-offset, which is used to indent subexpressions within a larger expression. You can see that behavior in the above based on how the parenthesis are indented relative to the start of the expression. I've recently thought that is probably not what people are expecting so I was planning to default this value to 0. If, so this ends up looking like this:

   for Source_Dirs use
     Common.Paths &
     (
      "adainclude",
      "rp2350_svd",
      FreeRTOS.Source,
      FreeRTOS.Portable_Base & "RISC-V"
     );

   for Switches ("s-traceb.adb") use
     ALL_ADAFLAGS & ("-g")
     & NO_SIBLING_ADAFLAGS & ("-fno-inline-functions-called-once");

I don’t say there’s anything wrong, just something I’ll need to get used to

I'm open to opinions on how you think this should behave. I'm working on indentation support for ada-ts-mode as well, and these types of decisions would flow into that too. I thought I was making an improvement over what was provided in gpr-mode, but if it's not what people expect, it might not be an improvement.

.. well worth the effort, thank you!

Thanks, I'm glad you find it useful!

Looking at the second example, does gpr-ts-mode have a line length (like the 79 in ada-mode) that it’ll automatically fold long lines to, or does it require the user to insert newlines where they want the folds?

Currently, there is nothing in the mode to support folding lines. I could look into that, but it hadn't crossed my mind.

simonjwright commented 1 month ago

I think I’d keep gpr-ts-mode-indent-exp-item-offset, set to 2. It’s one of those things where sometimes, even within the same file, some cases feel better one way, some the other. The operative word being "feel".

Currently, there is nothing in the mode to support folding lines. I could look into that, but it hadn't crossed my mind.

Sometimes, with ada-mode, you’d type the final newline of a section and the whole section would re-format itself, folding included!

And, automatically choosing where the folds go feels rather overbearing. So I wouldn’t worry.

brownts commented 1 month ago

Thanks for the input, I'll keep the default for gpr-ts-mode-indent-exp-item-offset.

While it doesn't address folding, I did release a new version of gpr-ts-mode yesterday that supports declaration-based indentation (now the new default). This will attempt to re-indent an entire declaration, if there are no syntax errors in the declaration. In the presence of syntax errors, it will fallback to the normal line-based indentation. The intention of this is to recover from previously incorrect indentation due to the presence of syntax errors. You can go back to the previous indentation strategy by changing gpr-ts-mode-indent-strategy to line.

I'm hoping this will be an improvement over the previous line-based indentation which would leave the buffer needing to be reformatted when multi-line constructs (i.e., case construction , package declaration, project declarations, etc.) were not syntactically correct when indentation was performed.

If you don't mind me asking, how did you enable/configure folding? I poked around at the gpr-mode code, but didn't see anything to configure, nor did I find documentation on it, but maybe I just wasn't looking in the correct location.

simonjwright commented 3 weeks ago

The new strategy feels very comfortable.

I didn’t write the code for folding in gpr-mode (or ada-mode; the same engine, I believe). Long ago I wrote a formatter for Coral 66, which had concepts of single & parallel fold points (the parallel ones were for e.g. parameter lists, so that if one had to fold they all did). The engine laid out the output line from the left, laying exception handlers at the fold points; when it hit the end of the line, it raised an exception; the lowest handler would then convert itself to a fold (if parallel, ditto for all the siblings); start again at the beginning; repeat until the end of the input line is reached, then dump the line with the folds. I don’t remember how the indentation was managed, this was somewhere about 1995!

One point - it turns out that the SPARK tool gnatprove has its own GPR package Prove. gpr-ts-mode handles it OK, even though it’s not in gpr-ts-mode-package-names.

brownts commented 3 weeks ago

The new strategy feels very comfortable.

I personally thought it dramatically improved the indentation situation, so glad to hear you find it works well too.

I didn’t write the code for folding in gpr-mode (or ada-mode; the same engine, I believe). Long ago I wrote a formatter for Coral 66, which had concepts of single & parallel fold points (the parallel ones were for e.g. parameter lists, so that if one had to fold they all did). The engine laid out the output line from the left, laying exception handlers at the fold points; when it hit the end of the line, it raised an exception; the lowest handler would then convert itself to a fold (if parallel, ditto for all the siblings); start again at the beginning; repeat until the end of the input line is reached, then dump the line with the folds. I don’t remember how the indentation was managed, this was somewhere about 1995!

That's quite interesting. I had to look up Coral 66 as I had never heard of it before. Sounds like it was at least somewhat inspirational for what became Ada. I feel like the wrapping engine you describe is partially how the indentation engine works (from a top-down perspective). I do wonder if the wrapping part might be covered by the Emacs auto-fill-mode.

I am curious if the following might get you close to what you're expecting. I thought that maybe ada-mode might have enabled auto-fill-mode by default, but when I started it up, I didn't notice that it did. The following is something you could try in a GPR buffer to see if this meets your expectation.

M-: (setq-local fill-indent-according-to-mode t)
M-: (setq-local fill-column 80)
M-x auto-fill-mode

One point - it turns out that the SPARK tool gnatprove has its own GPR package Prove. gpr-ts-mode handles it OK, even though it’s not in gpr-ts-mode-package-names.

Thanks, I'll add it to the list in the next release. That variable is currently only used to adjust the syntax highlighting for variable/attribute/package references. For example something like "A.B.C'Attribute", it's ambiguous if "C" is a reference to a package in project "A.B" or if it is a reference to a child project. Since package names get special syntax highlighting, gpr-ts-mode-package-names is used to resolve the ambiguity in the reference.

As a side note, I'm working on improving the indentation in the presence of syntax errors. It tries to match the nearest "case", "package", "project", etc. in order to anchor the indentation, when there are syntax errors. So far, it works quite well when writing a GPR from a top-down perspective.

Additionally, I'm working on some completion functionality. Currently I have package/project names after "end" (which completes for the name at the beginning of the project/package) as well as the name for packages (which pulls it's suggestions from gpr-ts-mode-package-names). I'm not sure if you use the LSP to provide completions for your GPR files, but the current version doesn't seem to provide completions after "end". Therefore, I've added this in as it was annoying me.

Just as quick demonstration, the following shows the behavior in my current work-in-progress updates (not yet officially available):

output-2024-11-09-21:10:46