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.

Sign up for GitHub

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

Add MarkerMode to select a MarkerPrinter #4799

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add MarkerMode to select a MarkerPrinter #4799

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7fde385
Add `MarkerMode` to select a `MarkerPrinter`
pstreef Dec 19, 2024
66dc881
Update names and add comments
pstreef Dec 19, 2024
7c16d38
fix compilation
pstreef Dec 19, 2024
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
64 changes: 64 additions & 0 deletions rewrite-core/src/main/java/org/openrewrite/PrintOutputCapture.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
*/
package org.openrewrite;

import lombok.Getter;
import lombok.RequiredArgsConstructor;
import org.jspecify.annotations.Nullable;
import org.openrewrite.marker.Marker;
import org.openrewrite.marker.Markup;
import org.openrewrite.marker.SearchResult;

import java.util.function.UnaryOperator;

Expand Down Expand Up @@ -67,6 +71,42 @@ public PrintOutputCapture<P> clone() {
}

public interface MarkerPrinter {

@Incubating(since = "8.41.4")
@RequiredArgsConstructor
enum MarkerMode {
Copy link
Member

@jkschneider jkschneider Dec 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I know naming is hard, but I don't love the name. I think there is a similar design concept in java.nio.charset. There is a Charset abstract class and a separate top level class called StandardCharsets. I think a top level StandardMarkerPrinters might be a slightly better design.

/**
* Does not print any markers.
*/
NONE(MarkerPrinter.NONE),
/**
* Prints a squiggly line arrow in front of the marked element wrapped in a comment.
* /*~~>* /Thing thing
*/
DEFAULT(MarkerPrinter.DEFAULT),
/**
* Prints a squiggly line arrow in front of the marked element wrapped in a comment.
* Possibly adding verbose information, depending on the marker
* /*~~>(this is some verbose information)* /Thing thing
*/
VERBOSE(MarkerPrinter.VERBOSE),
/**
* Prints a squiggly arrow in front and after the marked element. It only includes {@link SearchResult}
* /*~~>* /Thing thing /**~~>* /
*/
SEARCH_ONLY(MarkerPrinter.SEARCH_ONLY),
/**
* Prints a fenced marker ID in front and after the marked element. It only includes {@link Markup} and {@link SearchResult}
* /*{{3e2a36bb-7c16-4b03-bdde-bffda08838e7}}* /Thing thing /*{{3e2a36bb-7c16-4b03-bdde-bffda08838e7}}* /
*/
FENCED_MARKUP_AND_SEARCH(MarkerPrinter.FENCED_MARKUP_AND_SEARCH);
@Getter
private final MarkerPrinter printer;
}

MarkerPrinter NONE = new MarkerPrinter() {
};

MarkerPrinter DEFAULT = new MarkerPrinter() {
@Override
public String beforeSyntax(Marker marker, Cursor cursor, UnaryOperator<String> commentWrapper) {
Expand All @@ -81,6 +121,30 @@ public String beforeSyntax(Marker marker, Cursor cursor, UnaryOperator<String> c
}
};

MarkerPrinter FENCED_MARKUP_AND_SEARCH = new MarkerPrinter() {
@Override
public String beforeSyntax(Marker marker, Cursor cursor, UnaryOperator<String> commentWrapper) {
return marker instanceof SearchResult || marker instanceof Markup ? "{{" + marker.getId() + "}}" : "";
}

@Override
public String afterSyntax(Marker marker, Cursor cursor, UnaryOperator<String> commentWrapper) {
return marker instanceof SearchResult || marker instanceof Markup ? "{{" + marker.getId() + "}}" : "";
}
};

MarkerPrinter SEARCH_ONLY = new MarkerPrinter() {
@Override
public String beforeSyntax(Marker marker, Cursor cursor, UnaryOperator<String> commentWrapper) {
return marker instanceof SearchResult ? marker.print(cursor, commentWrapper, false) : "";
}

@Override
public String afterSyntax(Marker marker, Cursor cursor, UnaryOperator<String> commentWrapper) {
return marker instanceof SearchResult ? marker.print(cursor, commentWrapper, false) : "";
}
};

default String beforePrefix(Marker marker, Cursor cursor, UnaryOperator<String> commentWrapper) {
return "";
}
Expand Down
10 changes: 10 additions & 0 deletions rewrite-core/src/main/java/org/openrewrite/Result.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,16 @@ public String diff(@Nullable Path relativeTo, PrintOutputCapture.@Nullable Marke
return diff(relativeTo, markerPrinter, false);
}

@Incubating(since = "8.41.4")
Copy link
Member

Choose a reason for hiding this comment

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

If we move this StandardMarkerPrinters then we don't need to add these overloads. Indeed I would prefer we don't add more overload variations here unless absolutely necessary. Every new overload is one more item in the IDE autocomplete list.

public String diff(@Nullable Path relativeTo, PrintOutputCapture.MarkerPrinter.MarkerMode markerMode) {
return diff(relativeTo, markerMode, false);
}

@Incubating(since = "8.41.4")
public String diff(@Nullable Path relativeTo, PrintOutputCapture.MarkerPrinter.MarkerMode markerMode, boolean ignoreAllWhitespace) {
return diff(relativeTo, markerMode.getPrinter(), ignoreAllWhitespace);
}

@Incubating(since = "7.34.0")
public String diff(@Nullable Path relativeTo, PrintOutputCapture.@Nullable MarkerPrinter markerPrinter, @Nullable Boolean ignoreAllWhitespace) {
Path beforePath = before == null ? null : before.getSourcePath();
Expand Down
Loading