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.

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 term bank reference inSyntaxError; add character reference in ExtensionError #1019

Closed
michel-ZH opened this issue May 31, 2024 · 7 comments
Labels
good first issue The issue is a good starting point for first-time contributors kind/enhancement The issue or PR is a new feature or request triage/accepted The issue or PR is ready to be actively worked on

Comments

@michel-ZH
Copy link

Currently yomitan only tells you which line and column has a syntax error validating dictionary files. It would be nice to also know which term bank this applies to.

@michel-ZH michel-ZH added the kind/enhancement The issue or PR is a new feature or request label May 31, 2024
@michel-ZH
Copy link
Author

There are actually two error messages which could be made more verbose:
The issue outlined above
term_bank_missing
And the opposite issue, when the file is referenced but not the string causing the issue
character_reference_missing

@michel-ZH michel-ZH changed the title Add term bank reference in the JSON SyntaxError warning Add term bank reference inSyntaxError; add character reference in ExtensionError May 31, 2024
@jamesmaa jamesmaa added triage/accepted The issue or PR is ready to be actively worked on good first issue The issue is a good starting point for first-time contributors labels Aug 30, 2024
@austinyu12
Copy link

Hi, I would like to attempt taking on this issue.

@austinyu12
Copy link

#1462 makes SyntaxError message more verbose. ExtensionError still needs to be made more verbose.

@austinyu12
Copy link

Modified the ExtensionError message to include schema.errors. I wonder if this is fine. I just noticed that there is also a key property inside schema.errors; Perhaps that is what I should return.
Screenshot 2024-10-07 at 7 46 54 PM

@austinyu12
Copy link

Enhancements completed.

@tomodutch
Copy link

Considering the PR is already merged
#1473

Is this issue solved?

@jamesmaa
Copy link
Collaborator

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue The issue is a good starting point for first-time contributors kind/enhancement The issue or PR is a new feature or request triage/accepted The issue or PR is ready to be actively worked on
Projects
None yet
Development

No branches or pull requests

4 participants