Feat/80 char line break

macintoshpie commented 3 years ago

Background

modelicafmt does not take line length into consideration. This would be helpful to prevent really long lines.

Changes

break on long lines (80+ chars). modelicafmt will now insert a newline (and indent one additional level relative to the parent line) if:
- the previous token is "breakable", including , | = | (
- the length of the terminal token + the length of its whitespace prefix + the number of chars already written to the line is greater than 80

The linebreak is "dumb" meaning that all it does is insert the newline and indentation. e.g. assume there's 80 chars before c

# before formatter
something(a,b,c,d,e,f,g)
# after formatter
something(a,b,
  c,d,e,f,g)

An ideal implementation (in my opinion) would probably do something like this:

# before formatter
something(a,b,c,d,e,f,g)
# after formatter
something(
  a,
  b,
  c,
  d,
  e,
  f,
  g,
)

There are also some edgecases it can't handle. E.g. if we have <insert 78 chars>=a.sub_component.y it will not break after the =, though we would probably expect that it would. We would like to consider a.sub_component.y a single "token"/"thing" but our current tokenizer does not currently (it considers them separate: "a", ".", "sub_component", ".", "y"). So it will write the "a", and by the time it wants to break (on the ".") we've passed our opportunity to break.

AntoineGautier commented 3 years ago

@macintoshpie This PR solves the most problematic issues with long annotations. I have a few remarks enumerated below by order of priority.

I would not use { as a breaking character: this is an array constructor that we would like to keep next to the enclosed constructing clause.

When considering the following formatting I wonder if we should not break lines only if the annotation exceeds 80 characters (with a margin, say 5-10 characters). The reason is that the model developer (or the development tool) may have organized properly the annotation, which the fomater will ruin, producing less readable code.

// Before formatter
partial model PartialTwoPort "Partial component with two ports"
  replaceable package Medium =
    Modelica.Media.Interfaces.PartialMedium "Medium in the component"
      annotation (choices(
        choice(redeclare package Medium = Buildings.Media.Air "Moist air"),
        choice(redeclare package Medium = Buildings.Media.Water "Water"),
        choice(redeclare package Medium =
            Buildings.Media.Antifreeze.PropyleneGlycolWater (
              property_T=293.15,
              X_a=0.40)
              "Propylene glycol water, 40% mass fraction")));

// After formatter
partial model PartialTwoPort
  "Partial component with two ports"
  replaceable package Medium=Modelica.Media.Interfaces.PartialMedium
    "Medium in the component"
    annotation (choices(choice(redeclare package Medium=Buildings.Media.Air "Moist air"),
      choice(redeclare package Medium=Buildings.Media.Water "Water"),choice(
      redeclare package Medium=Buildings.Media.Antifreeze.PropyleneGlycolWater(
      property_T=293.15,X_a=0.40) "Propylene glycol water, 40% mass fraction")));

// Before formatter
connector ModeTypeOutput = output CHPs.BaseClasses.Types.Mode
  "Output connector for mode type"
annotation (
  defaultComponentName="y",
  Icon(
    coordinateSystem(preserveAspectRatio=true,
      extent={{-100.0,-100.0},{100.0,100.0}}),
      graphics={
    Polygon(
      lineColor={0,127,0},
      fillColor={255,255,255},
      fillPattern=FillPattern.Solid,
      points={{-100.0,100.0},{100.0,0.0},{-100.0,-100.0}})}),
  Diagram(
    coordinateSystem(preserveAspectRatio=true,
      extent={{-100.0,-100.0},{100.0,100.0}}),
      graphics={
    Polygon(
      lineColor={0,127,0},
      fillColor={255,255,255},
      fillPattern=FillPattern.Solid,
      points={{-100.0,50.0},{0.0,0.0},{-100.0,-50.0}}),
    Text(
      lineColor={0,0,127},
      extent={{30.0,60.0},{30.0,110.0}},
      textString="%name")}),
  Documentation(info="<html>

// After formatter
connector ModeTypeOutput=output CHPs.BaseClasses.Types.Mode
  "Output connector for mode type"
  annotation (defaultComponentName="y",Icon(coordinateSystem(preserveAspectRatio=
    true,extent={{-100.0,-100.0},{100.0,100.0}}),graphics={Polygon(lineColor={0,
    127,0},fillColor={255,255,255},fillPattern=FillPattern.Solid,points={{-100.0,
    100.0},{100.0,0.0},{-100.0,-100.0}})}),Diagram(coordinateSystem(
    preserveAspectRatio=true,extent={{-100.0,-100.0},{100.0,100.0}}),graphics={
    Polygon(lineColor={0,127,0},fillColor={255,255,255},fillPattern=FillPattern.Solid,
    points={{-100.0,50.0},{0.0,0.0},{-100.0,-50.0}}),Text(lineColor={0,0,127},
    extent={{30.0,60.0},{30.0,110.0}},textString="%name")}),Documentation(info=
    "<html>

When formatting Buildings/Fluid/Types.mo

// Before formatter
type CvTypes = enumeration(
  OpPoint "flow coefficient defined by m_flow_nominal/sqrt(dp_nominal)",
  Kv "Kv (metric) flow coefficient",
  Cv "Cv (US) flow coefficient",
  Av "Av (metric) flow coefficient")

// After formatter
type CvTypes=enumeration(OpPoint
  "flow coefficient defined by m_flow_nominal/sqrt(dp_nominal)",Kv
  "Kv (metric) flow coefficient",Cv
  "Cv (US) flow coefficient",Av
  "Av (metric) flow coefficient")

Why is it treated differently than a standard declaration with line break after (?

Another variant of that behavior, which in addition leads to a line exceeding 80 characters.

// Before formatter
record Viessmann_BW301A29_37kW_6_0COP_R410A =
  Buildings.Fluid.HeatPumps.Data.ScrollWaterToWater.Generic (
    volRat = 2.17993091357,
    V_flow_nominal = 0.00519094827579,
    leaCoe = 0.00125206659019,
    etaEle = 0.796906164099,
    PLos = 99.9278169331,
    dTSup = 0.532774492232,
    UACon = 27989.764814,
    UAEva = 58762.7503506)

// After formatter
record Viessmann_BW301A29_37kW_6_0COP_R410A=Buildings.Fluid.HeatPumps.Data.ScrollWaterToWater.Generic(
  volRat=2.17993091357,
  V_flow_nominal=0.00519094827579,
  leaCoe=0.00125206659019,
  etaEle=0.796906164099,
  PLos=99.9278169331,
  dTSup=0.532774492232,
  UACon=27989.764814,
  UAEva=58762.7503506)

macintoshpie commented 3 years ago

@AntoineGautier Thanks for the feedback, I removed the curly right brace as a possible char to break after. Unfortunately your other request to preserve original whitespace is not addressable currently

@nllong let me know if you have any other thoughts or preferences on this.

nllong commented 3 years ago

@macintoshpie You want to merge and bump release version?

urbanopt / modelica-fmt

Feat/80 char line break #24

Background

Changes