AdaCore / libadalang-tools

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

gnatpp: Multiline generic package instantiations are not handled well #1

Open LordAro opened 5 years ago

LordAro commented 5 years ago

For example,

package p_pre_dominators is new dominators
   (t_node_id => t_internal_node_p,
    nb_succs => predom_nb_succs,
    get_succ => predom_get_succ,
    nb_preds => predom_nb_preds,
    get_pred => predom_get_pred,
    node_id_equal => "=",
    node_id_hash => node_id_hash,
    set_idom => predom_set_pre_idom);

is turned into:

package p_pre_dominators is new dominators (t_node_id => t_internal_node_p, nb_succs => predom_nb_succs,
   get_succ => predom_get_succ, nb_preds => predom_nb_preds, get_pred => predom_get_pred, node_id_equal => "=",
   node_id_hash                                       => node_id_hash, set_idom => predom_set_pre_idom);

I mean, it's tried to align the parameters, I guess. It'd be much nicer if it was able to retain the vertical structure as with the other --vertical-foo options (if it goes over the line limit)

Using the following invocation with Pro19-20181010, though I can't see the version or any of the options making a difference:

--max-line-length=120 --vertical-named-aggregates --vertical-enum-types --call-threshold=5 --par-threshold=5 --no-align-modes

ProudPenguin commented 5 years ago

Hi,

I got a new job at the beginning of this year and therefore switched from Java to Ada as my main programming language.

I have to admit that I was a bit shocked how limited the functionality of the pretty printer is/was (my employer uses an old version of GNAT Pro) but I found out that the functionality was improved in the latest version of the GNAT Community Edition. gnatpp --version displays GNATPP Community 2019 (20190517).

After a while of experimenting with the pretty printer options I found this issue opened by @LordAro.

I created this small code example:

with Ada.Containers.Vectors;

procedure Example is

   type Record_Type is record
      A : Natural;
      B : Boolean;
      C : Float;
      D : Boolean;
   end record;

   type Holder_Type is record
      A : Natural;
      B : Record_Type;
      C : Record_Type;
   end record;

   --

   type Record_Type_Access is access Record_Type;

   package Record_Vector is new Ada.Containers.Vectors (Index_Type => Natural, Element_Type => Record_Type);

   --

   My_Record : Record_Type := (A => 1, B => False, C => 42.0, D => True);

   My_Record_Access_One : access Record_Type := new Record_Type'(A => 1, B => False, C => 42.0, D => True);

   My_Record_Access_Two : Record_Type_Access := new Record_Type'(A => 1, B => False, C => 42.0, D => True);

   --

   My_Holder_One : Holder_Type := (A => 1, B => (A => 123, B => False, C => 456.0, D => True), C => (A => 1, B => True, C => 789.123, D => True));

   My_Holder_Two : Holder_Type := (1, (A => 123, B => False, C => 456.0, D => True), (A => 1, B => True, C => 789.123, D => True));
begin
   null;
end Example;

Using the pretty printer options --max-line-length=120 --vertical-named-aggregates the code is turned into:

with Ada.Containers.Vectors;

procedure Example is

   type Record_Type is record
      A : Natural;
      B : Boolean;
      C : Float;
      D : Boolean;
   end record;

   type Holder_Type is record
      A : Natural;
      B : Record_Type;
      C : Record_Type;
   end record;

   --

   type Record_Type_Access is access Record_Type;

   package Record_Vector is new Ada.Containers.Vectors (Index_Type => Natural, Element_Type => Record_Type);

   --

   My_Record : Record_Type :=
     (A => 1,
      B => False,
      C => 42.0,
      D => True);

   My_Record_Access_One : access Record_Type := new Record_Type'(A => 1, B => False, C => 42.0, D => True);

   My_Record_Access_Two : Record_Type_Access := new Record_Type'(A => 1, B => False, C => 42.0, D => True);

   --

   My_Holder_One : Holder_Type :=
     (A => 1,
      B =>
        (A => 123,
         B => False,
         C => 456.0,
         D => True),
      C =>
        (A => 1,
         B => True,
         C => 789.123,
         D => True));

   My_Holder_Two : Holder_Type :=
     (1,
      (A => 123,
       B => False,
       C => 456.0,
       D => True),
      (A => 1,
       B => True,
       C => 789.123,
       D => True));
begin
   null;
end Example;

I would like to know why the option --vertical-named-aggregates has no effect on the formatting of the lines with the vector package instanciation and the definition of the variables My_Record_Access_One and My_Record_Access_Two? Also setting --call_threshold=1 --par_threshold=1 has no effect.

In my opinion the two following hand formatted code parts are more readable (also imagine package instantiations with more parameters ...) :

package Record_Vector is new Ada.Containers.Vectors
   (Index_Type   => Natural,
    Element_Type => Record_Type);
My_Record_Access_One : access Record_Type := new Record_Type'
   (A => 1,
    B => False,
    C => 42.0,
    D => True);

Any chance that a new option could be introduced in the future to allow this special kind of formatting? ;)

ProudPenguin commented 4 years ago

I built the latest version of the tools today based on the master branch (6c9da9fb60d060147aa8c28df2514572799b3dce) because I noticed a few commits that could be related to my request.

I formatted my previous code example using the options --max-line-length=120 --vertical-named-aggregates. The current version of GNATPP Community 2019 (20190517) included in the GNAT Community Edition gives the following result:

   --

   My_Record : Record_Type :=
     (A => 1,
      B => False,
      C => 42.0,
      D => True);

   My_Record_Access_One : access Record_Type := new Record_Type'(A => 1, B => False, C => 42.0, D => True);

   My_Record_Access_Two : Record_Type_Access := new Record_Type'(A => 1, B => False, C => 42.0, D => True);

   --

The latest version from the master branch gives the following result:

   --

   My_Record : Record_Type :=
     (A => 1,
      B => False,
      C => 42.0,
      D => True);

   My_Record_Access_One : access Record_Type := new Record_Type'
       (A => 1,
        B => False,
        C => 42.0,
        D => True);

   My_Record_Access_Two : Record_Type_Access := new Record_Type'
       (A => 1,
        B => False,
        C => 42.0,
        D => True);

      --

I think the readability is improved. ;) But the indentation is not the same:

ProudPenguin commented 4 years ago

Any feedback on my spaces issue?

LordAro commented 1 year ago

I feel like this should not have been closed - doesn't look like the '#1' referenced in the above commit is not the same as this one? (Actually an internal gitlab issue?)

Fabien-Chouteau commented 1 year ago

Sorry this was closed by mistake.