Skip to content

Reduce the size of the match string in TextMatches data table#4874

Merged
MBoegers merged 11 commits intomainfrom
limited-find-context
May 7, 2025
Merged

Reduce the size of the match string in TextMatches data table#4874
MBoegers merged 11 commits intomainfrom
limited-find-context

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

When a pattern is matched against a very long line, the result in the data table is now truncated to include at most 50 characters before and 50 characters after the match of that line.

When a pattern is matched against a very long line, the result in the data table is now truncated to include at most 50 characters before and 50 characters after the match of that line.

TreeVisitor<?, ExecutionContext> visitor = new TreeVisitor<Tree, ExecutionContext>() {

static final int CONTEXT_SIZE = 50;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps this should be configurable? Sometimes you might want long matches

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I the context size is now optional configurable. The default is 0 leading to just the matched text in the Datatable.
With -1 the truncate can be deactivated and the full text is in the Datatable, as on main currently for whoever uses this.

@MBoegers
Copy link
Copy Markdown
Contributor

MBoegers commented May 6, 2025

Is it necessary to have the Search marker inside the Match text in the Datatable? if we add StartMatch and EndMatch we could have the same data without the strange looking ~~>

EDIT: this would break existing logic and looks like not a good idea at this point.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • rewrite-core/src/main/java/org/openrewrite/text/Find.java
    • lines 21-21

Comment thread rewrite-core/src/main/java/org/openrewrite/text/Find.java Outdated
Comment thread rewrite-core/src/main/java/org/openrewrite/text/Find.java Outdated
timtebeek and others added 2 commits May 7, 2025 11:56
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@timtebeek
Copy link
Copy Markdown
Member

Copying the failure here: new test expectation not updated it seems.

RewriteRpcTest > runRecipe() FAILED
    java.lang.AssertionError: 
    Expecting ArrayList:
      [TextMatches.Row(sourcePath=hello.txt, match=...~~>Hello...)]
    to contain:
      [TextMatches.Row(sourcePath=hello.txt, match=~~>Hello Jon!)]
    but could not find the following element(s):
      [TextMatches.Row(sourcePath=hello.txt, match=~~>Hello Jon!)]
        at org.openrewrite.rpc.RewriteRpcTest.lambda$runRecipe$4(RewriteRpcTest.java:137)
        at org.openrewrite.rpc.RewriteRpcTest$$Lambda$1088/0x00007f9e4040bbc8.acceptThrows(Unknown Source)
        at org.openrewrite.test.UncheckedConsumer.accept(UncheckedConsumer.java:27)
        at org.openrewrite.test.RecipeSpec.lambda$dataTable$5(RecipeSpec.java:213)
        at org.openrewrite.test.RecipeSpec$$Lambda$833/0x00007f9e40319458.acceptThrows(Unknown Source)
        at org.openrewrite.test.UncheckedConsumer.accept(UncheckedConsumer.java:27)
        at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:388)
        at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:130)
        at org.openrewrite.rpc.RewriteRpcTest.runRecipe(RewriteRpcTest.java:131)

@MBoegers MBoegers merged commit f358c66 into main May 7, 2025
2 checks passed
@MBoegers MBoegers deleted the limited-find-context branch May 7, 2025 15:47
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite May 7, 2025
@timtebeek
Copy link
Copy Markdown
Member

Please keep an eye out for downstream changes needed after these types of changes as well:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants