Skip to content

Improves error message for missing or invalid arguments for rangemap command#817

Open
Abigael-JT wants to merge 2 commits into
teragrep:mainfrom
Abigael-JT:issue_689
Open

Improves error message for missing or invalid arguments for rangemap command#817
Abigael-JT wants to merge 2 commits into
teragrep:mainfrom
Abigael-JT:issue_689

Conversation

@Abigael-JT

@Abigael-JT Abigael-JT commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Improves error message for missing or invalid arguments for rangemap command

Description

Checklists

Testing

General

  • I have checked that my test files and functions have meaningful names.
  • I have checked that each test tests only a single behavior.
  • I have done happy tests.
  • I have tested only my own code.
  • I have tested at least all public methods.

Assertions

  • I have checked that my tests use assertions and not runtime overhead.
  • I have checked that my tests end in assertions.
  • I have checked that there is no comparison statements in assertions.
  • I have checked that assertions are in tests and not in helper functions.
  • I have checked that assertions for iterables are outside of for loops and both sides of the iteration blocks.
  • I have checked that assertions are not tested inside consumers.

Testing Data

  • I have tested algorithms and anything else with the possibility of unbound growth.
  • I have checked that all testing data is local and fully replaceable or reproducible or both.
  • I have checked that all test files are standalone.
  • I have checked that all test-specific fake objects and classes are in the test directory.
  • I have checked that my tests do not contain anything related to customers, infrastructure or users.
  • I have checked that my tests do not contain non-generic information.
  • I have checked that my tests do not do external requests and are not privately or publicly routable.

Statements

  • I have checked that my tests do not use throws for exceptions.
  • I have checked that my tests do not use try-catch statements.
  • I have checked that my tests do not use if-else statements.

Java

  • I have checked that my tests for Java uses JUnit library.
  • I have checked that my tests for Java uses JUnit utilities for parameters.

Other

  • I have only tested public behavior and not private implementation details.
  • I have checked that my tests are not (partially) commented out.
  • I have checked that hand-crafted variables in assertions are used accordingly.
  • I have tested Object Equality.
  • I have checked that I do not have any manual tests or I have a valid reason for them and I have explained it in the PR description.

Code Quality

  • I have checked that my code follows metrics set in Procedure: Class Metrics.
  • I have checked that my code follows metrics set in Procedure: Method Metrics.
  • I have checked that my code follows metrics set in Procedure: Object Quality.
  • I have checked that my code does not have any NULL values.
  • I have checked my code does not contain FIXME or TODO comments.

@Abigael-JT Abigael-JT self-assigned this Feb 13, 2026
@Abigael-JT Abigael-JT linked an issue Feb 13, 2026 that may be closed by this pull request
@Abigael-JT Abigael-JT marked this pull request as ready for review February 16, 2026 14:24

@elliVM elliVM left a comment

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.

While we do fix some of the exception cases from parser in this change.
It seems to me that the visitor should not be responsible for handling parser syntax exceptions at all. It should interpret the parse tree with the assumption that the tree is structurally correct.

I think this exposes a common scenario in our current design where the pth_03 parser provides a broken or incomplete tree to the visitor. Result is that these parser failures leak into the visitors and further on into the exception messages.
Parser exception messages don't really make any sense to the end user as pointed out in the issue description.

No easy solution for this here but I think it is worth raising the problem.

See my suggestion for refactoring the pth_03 rule and if it makes sense to you please create an issue for it.


@Override
public Node visitT_rangemap_attrnParameter(DPLParser.T_rangemap_attrnParameterContext ctx) {
final String key = ctx.stringType().getText();

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.

We are currently building separate tokens for GET_NUMBER_LEFT and GET_NUMBER_RIGHT which leads to partial parse states that force defensive handling here.

It could be worth considering refactoring the pth_03 parser grammar to represent
range : NUMBER '-' NUMBER.

Then extraction becomes easy here

left = rangeCtx.NUMBER(0).getText()
right = rangeCtx.NUMBER(1).getText()

// parse to double
// ensure start <= end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a valid point, I have raised a ticket for this in PTH_03 teragrep/pth_03#120

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide a better error message when user typoes rangemap command

2 participants