Re: [Ada-mode-users] Indenting incomplete code during editing

[Stephen Leake]

Ludovic Brenta <address@hidden> writes:

> We have deployed ada-mode 5.2 as an option in our office and half a
> dozen people are now using it in lieu of ada-mode 4.0; we are starting
> to receive negative feedback about the indentation of code being
> edited.  

Ok.

<snip>
> Now, point is before the closing paren and the user types ; RET to
> insert a second parameter.  They expect the new line to be indented
> and point to be under the P of Param, like so with * standing for
> point:
<snip>

> Instead they get this:
>
> package body P is
> procedure G (Param : in Boolean;
> *) is
>   begin
>     Some_Statement;
>   end G;
> end P;

I agree this is not friendly; patch below.

GPS does this right.

> this, our user types the new parameter name:
>
> package body P is
> procedure G (Param : in Boolean;
> New_Param : in Integer) is
>   begin
>     Some_Statement;
>   end G;
> end P;
>
> and now has to press TAB to reindent both lines:
>
> package body P is
>   procedure G (Param : in Boolean;
>                New_Param : in Integer) is
>   begin
>     Some_Statement;
>   end G;
> end P;

Yes. At least it's only a single keystroke to restore the indentation.

> Is there a way to avoid:
> - the reindentation of the first line;
> - the bad indentation the the new line;
> - the requirement for an explicit TAB to reindent
>
> I think these questions go deeper than this particular example; they
> have to do with the choice of basing the indentation engine on a
> parser for a complete (in fact more than complete) Ada grammar.

Yes. The patch below expands the Ada grammar to allow empty
parameter_specification.  

This type of fix is easy to implement; it does slow down the parser a
little bit, since it adds a conflict.

I'm sure there are more places in the grammar where it would be
helpful to allow an empty token, so be sure to report specific cases of
bad indentation.

> Continuing on the example, suppose we now want to surround
> Some_Statement in an "if" statement. The user places point before the
> S and then types "if New_Param > 0 then RET" and gets:
>
> package body P is
>   procedure G (Param : in Boolean;
>                New_Param : in Integer) is
>   begin
>   if New_Param > 0 then
>   Some_Statement;
>   end G;
> end P;
>
> so both the "if" line and the "Some_Statement" line are badly
> indented; the "if" line in particular was properly indented before
> the user typed RET, so the feeling is that RET actively breaks
> formatting.  

Yes; the parser cannot recover from the missing "end if", so it gives
up, and the indentation engine also gives up.

GPS does this right.

One alternative here, which does take some getting used to, is to use
the skeleton expansion; typing i f C-e  gives:

   begin
      if  then
      elsif  then
      else
      end if;
      Some_Statements;
   end G;

Then cut and paste "Some_Statements;" (ie, the body of the "then"
branch) to where it belongs.

This allows navigating over a large portion of code to select the body
to be moved (ie over a block or loop), using the parser-based navigation
commands.


The skeleton package also allows selecting a region to be embedded in
the skeleton, then invoking the skeleton function directly.
For example:

select "Some_statements;"

type M-x ada-skel-if

That gives:

      if Some_Statements; then
elsif  then
else
end if;

This is not what we want, but it is easily fixed by editing the
skeleton. Currently it leaves point after the "if"; it can be left after
the "then" instead. Ideally, we'd like the choice of where to leave
point to depend on the method of invoking.


In general, the ada-mode skeletons insert code that is accepted by the
parser, precisely to avoid bad indentation when adding code like this.

> I think half of the annoyance could be avoided by simply preserving
> the indentation of the line where the user typed RET.

That's a good idea; I'll see if I can do that.

> The other half is choosing a sensible indentation for the line
> containing point after the RET, based only on the previous line. 

That is the approach Ada mode 4.0 used. It becomes extremely unweildy
when you try to cover all the cases; that's why Emacs Ada mode 5.0 and
GPS use parser-based indentation.

It would be possible to handle a few special cases. but there would
always be pressure to handle more, so I'd rather not go down that road.

> But I might be wrong. Any opinions? Suggestions? Solutions?

There are two approaches that make sense here; defining two (or more)
levels of grammar, and adding intelligent (grammar-completing) error
handling to the parser.

GPS uses a third approach; a recursive-descent parser. That allows
halting the parse at any point, while providing complete parse
information before that point. Recursive-descent parsers are not
generated from grammars; they are written by hand. I wanted a parser
that was at least close to the defined language grammer (to make it easy
to adapt when the language changes), and adaptable to other languages
(gpr, in particular).

For two levels of grammar, we'd define the top level to contain only
block keywords; declare/begin/end, if/then/else/end if, etc. Everything
else would be treated as filler. The lower level would be the current
complete grammar, but it would be applied inside each high-level block
separately.

This strategy is used by the semantic-based parsers in some other Emacs
language modes; typically, the high level grammar is { }, which does not
work well for Ada ):.

This would _not_ handle adding "if" without "end if", but it would
isolate the bad indentation to the enclosing block.


For grammar-completing error handling, try to add the missing tokens to
complete the current grammar statement; add an implicit "end if".

The two approaches could be combined; it might be easier to decide what
tokens to add in the high level grammar.

I have not done any work towards either approach.

-- 
-- Stephe

#
# old_revision [468fd356bb69233e83bc01c729975a3987b5c76a]
#
# patch "ada-grammar.wy"
#  from [ed44910c83b62e290b2e2b577017cfcfb8a5f430]
#    to [a14bf1ea48a1c00e77b02753cddd519a10938de9]
#
============================================================
--- ada-grammar.wy      ed44910c83b62e290b2e2b577017cfcfb8a5f430
+++ ada-grammar.wy      a14bf1ea48a1c00e77b02753cddd519a10938de9
@@ -183,6 +183,7 @@
 %conflict REDUCE/REDUCE in state expression_opt, association_opt on token IS
 %conflict REDUCE/REDUCE in state expression_opt, association_opt on token 
RIGHT_PAREN
 %conflict REDUCE/REDUCE in state expression_opt, association_opt on token 
SEMICOLON
+%conflict REDUCE/REDUCE in state expression_opt, parameter_specification on 
token RIGHT_PAREN
 %conflict REDUCE/REDUCE in state identifier_list, name on token COMMA
 %conflict REDUCE/REDUCE in state name, direct_name on token USE
 %conflict REDUCE/REDUCE in state primary, subtype_indication on token COMMA
@@ -1740,7 +1741,8 @@ parameter_specification
   ;
 
 parameter_specification
-  : identifier_list COLON aliased_opt mode_opt null_exclusion_opt name 
COLON_EQUAL expression
+  : ;; empty
+  | identifier_list COLON aliased_opt mode_opt null_exclusion_opt name 
COLON_EQUAL expression
     (wisi-face-action [6 font-lock-type-face])
   | identifier_list COLON aliased_opt mode_opt null_exclusion_opt name
     (wisi-face-action [6 font-lock-type-face])