AdaCore / libadalang-tools

Libadalang-based tools
GNU General Public License v3.0
16 stars 13 forks source link

gnatpp: Feature suggestion #11

Closed ProudPenguin closed 4 years ago

ProudPenguin commented 4 years ago

Example Ada code formatted using gnatpp --call_threshold=1 input.adb:

procedure Main is
begin
   My_Package.Procedure_Name
     (First_Param   => "Example one",
      Another_Param => (1 => Consetetur));

   My_Package.Procedure_Name
     (First_Param   => "Example two",
      Another_Param => (1 => Consetetur, 2 => Sadipscing));

   My_Package.Procedure_Name
     (First_Param   => "Example three",
      Another_Param => (1 => Consetetur, 2 => Sadipscing, 3 => Elitr));

   My_Package.Procedure_Name
     (First_Param   => "Example four",
      Another_Param =>
        (1 => Consetetur, 2 => Sadipscing, 3 => Elitr, 4 => Sed));

   My_Package.Procedure_Name
     (First_Param   => "Example five",
      Another_Param =>
        (1 => Consetetur, 2 => Sadipscing, 3 => Elitr, 4 => Sed, 5 => Diam));

   My_Package.Procedure_Name
     (First_Param   => "Example six",
      Another_Param =>
        (1 => Consetetur_Sadipscing_Elitr_Sadipscing,
         2 => Elitr_Sed_Diam_Nonumy));
end Main;

Why not add an option to force each element of the arrays (parameter Another_Param) on a new line like in the last example (otherwise the line would become to long):

   My_Package.Procedure_Name
     (First_Param   => "Example six",
      Another_Param =>
        (1 => Consetetur_Sadipscing_Elitr_Sadipscing,
         2 => Elitr_Sed_Diam_Nonumy));

Expected output:

procedure Main is
begin
   My_Package.Procedure_Name
     (First_Param   => "Example one",
      Another_Param => (1 => Consetetur));

   My_Package.Procedure_Name
     (First_Param   => "Example two",
      Another_Param =>
        (1 => Consetetur,
         2 => Sadipscing));

   My_Package.Procedure_Name
     (First_Param   => "Example three",
      Another_Param =>
        (1 => Consetetur,
         2 => Sadipscing,
         3 => Elitr));

   My_Package.Procedure_Name
     (First_Param   => "Example four",
      Another_Param =>
        (1 => Consetetur,
         2 => Sadipscing,
         3 => Elitr,
         4 => Sed));

   My_Package.Procedure_Name
     (First_Param   => "Example five",
      Another_Param =>
        (1 => Consetetur,
         2 => Sadipscing,
         3 => Elitr,
         4 => Sed,
         5 => Diam));

   My_Package.Procedure_Name
     (First_Param   => "Example six",
      Another_Param =>
        (1 => Consetetur_Sadipscing_Elitr_Sadipscing,
         2 => Elitr_Sed_Diam_Nonumy));
end Main;

Maybe this change is as simple as updating/replacing a few defined templates (replace # with $)?

ArnaudCharlet commented 4 years ago

Why not indeed, with the same reserved as for #10 (proliferation of options). Another possible option would be a general --favor-line-breaks-over-long-lines which would basically systematically break lines after each parameter, condition, aggregate element, subprogram call, ...

ProudPenguin commented 4 years ago

Thank you for your answer. ;)

I tried to solve the problem myself. I found the following line:

https://github.com/AdaCore/libadalang-tools/blob/af084d02fabe184fa39d433d7db14fac7e3e0176/src/pp-actions.adb#L1572

And replaced it by:

Nonvertical_Agg_Alt => L ("$(?~~ with #~?~,$ ~~)"),

The example procedure calls two to six are formatted as expected but unfortunately this change also introduces a new line in the first procedure call:

   My_Package.Procedure_Name
     (First_Param   => "Example one",
      Another_Param =>
        (1 => Consetetur));

I figured out that this template definition is only used in the following procedure: https://github.com/AdaCore/libadalang-tools/blob/af084d02fabe184fa39d433d7db14fac7e3e0176/src/pp-actions.adb#L3482-L3489

My question: Is Tree a representation of (1 => Consetetur) (a tree with one child node)?

I am thinking about something like this:

 procedure Do_Aggregate is 
 begin 
    if Is_Vertical_Aggregate (Tree) then 
       Interpret_Alt_Template (Vertical_Agg_Alt); 
    else
       -- Question: Is such a function available?
       if Number_Of_Children (Tree) = 1 then
          Interpret_Alt_Template (Nonvertical_Agg_Alt);
          -- old template version: L ("#(?~~ with #~?~,# ~~)")
       else
          Interpret_Alt_Template (Nonvertical_Agg_Alt_New);
          -- new template version: L ("$(?~~ with #~?~,$ ~~)")
       end if; 
    end if; 
 end Do_Aggregate; 
ArnaudCharlet commented 4 years ago

Happy New Year! I'd suggest giving the latest sources a try where we've introduced the --no-compact switch which is likely a better starting point (and hopefully a complete solution) for you, let us know.

ArnaudCharlet commented 4 years ago

Assuming --no-compact is addressing the need.