Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 578 #591

Merged
merged 6 commits into from
Oct 22, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/dfmt/ast_info.d
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ struct ASTInformation
(structInfoSortedByEndLocation);
sort(ufcsHintLocations);
ufcsHintLocations = ufcsHintLocations.uniq().array();
sort(ternaryColonLocations);
}

/// Locations of end braces for struct bodies
Expand Down Expand Up @@ -135,6 +136,9 @@ struct ASTInformation

/// Opening & closing braces of struct initializers
StructInitializerInfo[] structInfoSortedByEndLocation;

/// Locations ternary expression colons.
size_t[] ternaryColonLocations;
}

/// Collects information from the AST that is useful for the formatter
Expand Down Expand Up @@ -438,6 +442,11 @@ final class FormatVisitor : ASTVisitor
outStatement.accept(this);
}

override void visit(const TernaryExpression ternaryExpression)
{
astInformation.ternaryColonLocations ~= ternaryExpression.colon.index;
}

private:
ASTInformation* astInformation;
}
3 changes: 2 additions & 1 deletion src/dfmt/formatter.d
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ private:
current.line);
immutable bool isStructInitializer = astInformation.structInfoSortedByEndLocation
.canFind!(st => st.startLocation < current.index && current.index < st.endLocation);
immutable bool isTernary = astInformation.ternaryColonLocations.canFindIndex(current.index);

if (isCase || isAttribute)
{
Expand All @@ -856,7 +857,7 @@ private:
newline();
}
}
else if (indents.topIs(tok!"]")) // Associative array
else if (indents.topIs(tok!"]") && !isTernary) // Associative array
{
write(config.dfmt_space_before_aa_colon ? " : " : ": ");
++index;
Expand Down
43 changes: 43 additions & 0 deletions tests/allman/issue0578.d.ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
void f()
{
auto t = true ? 1 : 0;
auto a = [true ? 1 : 0];
auto aa1 = [0: true ? 1 : 0];
auto aa2 = [0: true ? (false ? 1 : 2) : 3];

auto aa3 = [0: true ? false ? 1: 2 : 3];
Copy link
Member

@WebFreak001 WebFreak001 Oct 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be a part of the issue you said would be fixed in the title, no?

Still got broken formatting here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad I misunderstood libdparse, I fix it

/+
READ IF THIS TEST FAILS

Bug in dparse:
(Formatting before fix issue 578)
int[int] aa3 = [0: true ? false ? 1: 2: 3];
^

EXPLANATION:
The marked colon is not is not recognized as a TernaryExpression by
dparse:
If you write a `writeln(ternaryExpression.colon.index)` in the overloaded
ASTInformation visit function, which should get called once for every
encountered ternary colon, only the second index is printed:
override void visit(const TernaryExpression ternaryExpression) { ... }

This bug can be ignored (formatting error is localized and should be rarely
encountered).


FIX:
Should this get fixed by dparse or when the migration to dmd is completed,
the formatting in the .ref files can be updated to the correct one and this
comment can be removed.


Current formatting after applying fix issue 578:
auto aa3 = [0: true ? false ? 1: 2 : 3];
^

Correct formatting after fix dparse:
auto aa3 = [0: true ? false ? 1 : 2 : 3];
^
+/
}
43 changes: 43 additions & 0 deletions tests/issue0578.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
void f()
{
auto t = true ? 1 : 0;
auto a = [true ? 1: 0];
auto aa1 = [0: true ? 1: 0];
auto aa2 = [0: true ? (false ? 1: 2): 3];

auto aa3 = [0: true ? false ? 1: 2: 3];
/+
READ IF THIS TEST FAILS

Bug in dparse:
(Formatting before fix issue 578)
int[int] aa3 = [0: true ? false ? 1: 2: 3];
^

EXPLANATION:
The marked colon is not is not recognized as a TernaryExpression by
dparse:
If you write a `writeln(ternaryExpression.colon.index)` in the overloaded
ASTInformation visit function, which should get called once for every
encountered ternary colon, only the second index is printed:
override void visit(const TernaryExpression ternaryExpression) { ... }

This bug can be ignored (formatting error is localized and should be rarely
encountered).


FIX:
Should this get fixed by dparse or when the migration to dmd is completed,
the formatting in the .ref files can be updated to the correct one and this
comment can be removed.


Current formatting after applying fix issue 578:
auto aa3 = [0: true ? false ? 1: 2 : 3];
^

Correct formatting after fix dparse:
auto aa3 = [0: true ? false ? 1 : 2 : 3];
^
+/
}
43 changes: 43 additions & 0 deletions tests/knr/issue0578.d.ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
void f()
{
auto t = true ? 1 : 0;
auto a = [true ? 1 : 0];
auto aa1 = [0: true ? 1 : 0];
auto aa2 = [0: true ? (false ? 1 : 2) : 3];

auto aa3 = [0: true ? false ? 1: 2 : 3];
/+
READ IF THIS TEST FAILS

Bug in dparse:
(Formatting before fix issue 578)
int[int] aa3 = [0: true ? false ? 1: 2: 3];
^

EXPLANATION:
The marked colon is not is not recognized as a TernaryExpression by
dparse:
If you write a `writeln(ternaryExpression.colon.index)` in the overloaded
ASTInformation visit function, which should get called once for every
encountered ternary colon, only the second index is printed:
override void visit(const TernaryExpression ternaryExpression) { ... }

This bug can be ignored (formatting error is localized and should be rarely
encountered).


FIX:
Should this get fixed by dparse or when the migration to dmd is completed,
the formatting in the .ref files can be updated to the correct one and this
comment can be removed.


Current formatting after applying fix issue 578:
auto aa3 = [0: true ? false ? 1: 2 : 3];
^

Correct formatting after fix dparse:
auto aa3 = [0: true ? false ? 1 : 2 : 3];
^
+/
}
42 changes: 42 additions & 0 deletions tests/otbs/issue0578.d.ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
void f() {
auto t = true ? 1 : 0;
auto a = [true ? 1 : 0];
auto aa1 = [0: true ? 1 : 0];
auto aa2 = [0: true ? (false ? 1 : 2) : 3];

auto aa3 = [0: true ? false ? 1: 2 : 3];
/+
READ IF THIS TEST FAILS

Bug in dparse:
(Formatting before fix issue 578)
int[int] aa3 = [0: true ? false ? 1: 2: 3];
^

EXPLANATION:
The marked colon is not is not recognized as a TernaryExpression by
dparse:
If you write a `writeln(ternaryExpression.colon.index)` in the overloaded
ASTInformation visit function, which should get called once for every
encountered ternary colon, only the second index is printed:
override void visit(const TernaryExpression ternaryExpression) { ... }

This bug can be ignored (formatting error is localized and should be rarely
encountered).


FIX:
Should this get fixed by dparse or when the migration to dmd is completed,
the formatting in the .ref files can be updated to the correct one and this
comment can be removed.


Current formatting after applying fix issue 578:
auto aa3 = [0: true ? false ? 1: 2 : 3];
^

Correct formatting after fix dparse:
auto aa3 = [0: true ? false ? 1 : 2 : 3];
^
+/
}
Loading