-
Notifications
You must be signed in to change notification settings - Fork 142
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
feat(#3742): Split ParsingErrors
on Two Separate Classes
#3751
Conversation
@yegor256 Could you review these changes, please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@volodya-lombrozo looks interesting, a few comment above
eo-parser/src/main/java/org/eolang/parser/errors/ErrorDirectives.java
Outdated
Show resolved
Hide resolved
eo-parser/src/main/java/org/eolang/parser/errors/EoParserErrors.java
Outdated
Show resolved
Hide resolved
eo-parser/src/main/java/org/eolang/parser/errors/ParsingErrors.java
Outdated
Show resolved
Hide resolved
@yegor256 Could you have a look one more time, please? |
@volodya-lombrozo the naming of classes is rather confusing here: they look almost the same. Why don't you use the names suggested in the puzzle: |
@yegor256 I agree that the naming here is confusing. Let me try to explain. Before that PR, we had only one error listener: |
@yegor256 Recent changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@volodya-lombrozo it's much better now! please, see my comments above
@volodya-lombrozo can't you simply calculate the size of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@volodya-lombrozo thanks! |
In this PR I:
errors
for all error handlers and realted classes.ParsingErrors
onEoParserErrors
andParsingErrors
Lines
andErrorDirectives
to share common logic across all the new classes.When this PR seems having a lot of lines, it just moves some parts of the code across the codebase.
Closes: #3742.