Skip to content

Commit

Permalink
Reformat Variable hover context and array dimensions (#476)
Browse files Browse the repository at this point in the history
closes #475 

- Array dimensions went from a section inline into the variable type
- Context of the variable now includes all groups and array dimensions
  • Loading branch information
MarkusAmshove committed May 22, 2024
1 parent be16a34 commit 4fa3e88
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -221,17 +221,6 @@ private Hover hoverVariable(IVariableNode variable, HoverContext context)
var declaration = formatVariableDeclaration(context.file().module(), variable);
contentBuilder.appendCode(declaration.declaration);

if (variable.isArray())
{
contentBuilder.appendSection("dimensions", nested ->
{
for (var dimension : variable.dimensions())
{
nested.appendBullet(dimension.displayFormat());
}
});
}

var pathToVariableHover = VariableContextHover.create(context, variable);
pathToVariableHover.addVariableContext(contentBuilder);

Expand All @@ -243,7 +232,7 @@ private VariableDeclarationFormat formatVariableDeclaration(INaturalModule modul
var declaration = "%s %d %s".formatted(variable.scope().toString(), variable.level(), variable.name());
if (variable instanceof ITypedVariableNode typedVariableNode)
{
declaration += " %s".formatted(typedVariableNode.type().toShortString());
declaration += " %s".formatted(typedVariableNode.formatTypeForDisplay());
if (typedVariableNode.type().initialValue() != null)
{
var initValue = typedVariableNode.type().initialValue()instanceof ITokenNode tokenNode
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.amshove.natls.hover;

import org.amshove.natls.markupcontent.IMarkupContentBuilder;
import org.amshove.natparse.NodeUtil;
import org.amshove.natparse.natural.IHasDefineData;
import org.amshove.natparse.natural.INaturalModule;
import org.amshove.natparse.natural.IUsingNode;
Expand All @@ -24,10 +23,10 @@ public void addVariableContext(IMarkupContentBuilder builder)
var variableContext = new StringBuilder();

appendUsingComment(variableContext);
if (variable.level() > 1)
var parents = variable.getVariableParentsAscending();
for (var parent : parents)
{
var group = NodeUtil.findLevelOneParentOf(variable);
appendVariableComment(group, variableContext, true);
appendVariableComment(parent, variableContext, true);
}

appendVariableComment(variable, variableContext, !variableContext.isEmpty());
Expand All @@ -53,6 +52,26 @@ private void appendVariableComment(IVariableNode variable, StringBuilder chain,
{
var source = "%d %s".formatted(variable.level(), variable.name());
var comment = declaringModule.extractLineComment(variable.position().line());

if (variable.isArray())
{
var formattedDimensions = new StringBuilder();
for (var dimension : variable.dimensions())
{
if (dimension.parent() == variable)
{
if (!formattedDimensions.isEmpty())
{
formattedDimensions.append(",");
}
formattedDimensions.append(dimension.displayFormat());
}
}
if (!formattedDimensions.isEmpty())
{
source += " (%s)".formatted(formattedDimensions);
}
}
if (alwaysInclude || !comment.isEmpty())
{
appendComment("%s %s".formatted(source, comment), chain);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,25 @@ void arrayDimensionsShouldBeIncluded()
""",
"""
```natural
LOCAL 1 #MYVAR (A10)
```
LOCAL 1 #MYVAR (A10/1:*,1:5)
```"""
);
}

*dimensions:*
- 1:*
- 1:5"""
@Test
void dynamicAlphanumericArraysShouldBeFormatted()
{
assertHover(
"""
DEFINE DATA
LOCAL 1 #MY${}$VAR (A/1:*,1:5) DYNAMIC
END-DEFINE
END
""",
"""
```natural
LOCAL 1 #MYVAR (A/1:*,1:5) DYNAMIC
```"""
);
}

Expand Down Expand Up @@ -119,7 +132,7 @@ void theUsingDataAreaShouldBeIncludedInTheContext()
DEFINE DATA
LOCAL USING MYLDA
END-DEFINE
WRITE #MY${}$VAR
END""",
"""
Expand Down Expand Up @@ -150,7 +163,7 @@ void commentsOnTheUsingOfDataAreasShouldBeIncludedForGroupMembers()
DEFINE DATA
LOCAL USING MYLDA /* My using
END-DEFINE
WRITE #MY${}$VAR
END""",
"""
Expand Down Expand Up @@ -182,7 +195,7 @@ void allRelevantCommentsEncounteredOnTheWayToTheVariableShouldBeIncluded()
DEFINE DATA
LOCAL USING MYLDA /* My using
END-DEFINE
WRITE #MY${}$VAR
END""",
"""
Expand Down Expand Up @@ -224,6 +237,38 @@ void theLevelOneVariableShouldBeAddedIfHoveringANestedVariable()
);
}

@Test
void hoveringAVariableMultipleLevelsDownShouldShowTheCompleteContext()
{
assertHover(
"""
DEFINE DATA
LOCAL
1 #MYGROUP /* Comment 1
2 #GROUP2(1:*)
3 #GROUP3 /* Comment 2
4 #GROUP4(1:5) /* Comment 3
5 #VAR (A10)
END-DEFINE
WRITE #V${}$AR
END
""",
"""
```natural
LOCAL 5 #VAR (A10/1:*,1:5)
```
*context:*
```natural
1 #MYGROUP /* Comment 1
2 #GROUP2 (1:*)
3 #GROUP3 /* Comment 2
4 #GROUP4 (1:5) /* Comment 3
5 #VAR
```"""
);
}

@Test
void hoverOverVariablesInInputsShouldShowTheHover()
{
Expand Down
21 changes: 21 additions & 0 deletions libs/natparse/src/main/java/org/amshove/natparse/ReadOnlyList.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ private ReadOnlyList(Collection<T> collection)
this.collection = new ArrayList<>(collection);
}

/**
* Does NOT create a defensive copy.
*/
private ReadOnlyList(ArrayList<T> collection)
{
this.collection = collection;
}

public static <T> ReadOnlyList<T> from(Collection<T> collection)
{
if (collection == null)
Expand Down Expand Up @@ -74,6 +82,19 @@ public static <T> ReadOnlyList<T> ofExcludingNull(T... items)
return ReadOnlyList.from(includedItems);
}

/**
* Creates a new ReadOnlyList in reversed order.
*/
public ReadOnlyList<T> reverse()
{
var newList = new ArrayList<T>();
for (var i = collection.size() - 1; i >= 0; i--)
{
newList.add(collection.get(i));
}
return new ReadOnlyList<>(newList);
}

@Override
public @Nonnull Iterator<T> iterator()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package org.amshove.natparse.natural;

import java.util.stream.Collectors;

public interface ITypedVariableNode extends IVariableNode
{
IVariableType type();
Expand All @@ -18,10 +16,7 @@ default String formatTypeForDisplay()

if (isArray())
{
details += "/";
details += dimensions().stream()
.map(IArrayDimension::displayFormat)
.collect(Collectors.joining(", "));
details += "/" + formatDimensionList();
}

details += ")";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.amshove.natparse.natural;

import org.amshove.natparse.ReadOnlyList;
import java.util.*;
import java.util.stream.*;

public non-sealed interface IVariableNode extends IReferencableNode, IParameterDefinitionNode
{
Expand All @@ -24,4 +26,60 @@ default boolean isArray()
var dimensions = dimensions();
return dimensions != null && !dimensions.isEmpty();
}

/**
* Returns a list of all parent groups in descending (by level) order.
**/
default ReadOnlyList<IGroupNode> getVariableParentsDescending()
{
if (level() == 1)
{
return ReadOnlyList.empty();
}

var parents = new ArrayList<IGroupNode>();

var current = parent();
while (current != null)
{
if (current instanceof IGroupNode group)
{
parents.add(group);
}

current = current.parent();
}

return ReadOnlyList.from(parents);
}

/**
* Returns a list of all parent groups in ascending (by level) order.
**/
default ReadOnlyList<IGroupNode> getVariableParentsAscending()
{
if (level() == 1)
{
return ReadOnlyList.empty();
}

var parents = getVariableParentsDescending();
return parents.reverse();
}

/**
* Returns a formatted list of array dimensions without parens.<br>
* Example: `1:*,1:5`
*/
default String formatDimensionList()
{
if (!isArray())
{
return "";
}

return dimensions().stream()
.map(IArrayDimension::displayFormat)
.collect(Collectors.joining(","));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ void inheritDimensions(ReadOnlyList<IArrayDimension> dimensions)
@Override
void addDimension(ArrayDimension dimension)
{
addNode(dimension);
// If we get a new dimension that is defined for a parent variable, we don't take its ownership
if (dimension.parent() == null)
{
addNode(dimension);
}

if (dimension.isUpperVariable())
{
if (type == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,12 @@ void setScope(VariableScope scope)
void addDimension(ArrayDimension dimension)
{
dimensions.add(dimension);
addNode(dimension);

// If we get a new dimension that is defined for a parent variable, we don't take its ownership
if (dimension.parent() == null)
{
addNode(dimension);
}
}

/**
Expand All @@ -158,7 +163,7 @@ void inheritDimensions(ReadOnlyList<IArrayDimension> dimensions)
{
if (!this.dimensions.contains(dimension))
{
this.dimensions.add(0, dimension); // add inhereted dimensions first, as they're defined first
this.dimensions.addFirst(dimension); // add inhereted dimensions first, as they're defined first
}
}
}
Expand Down

0 comments on commit 4fa3e88

Please sign in to comment.