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

Jump to bottom

Only provide valid line/col info from Xerces #1364

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Only provide valid line/col info from Xerces #1364

wants to merge 2 commits into from
+7 −5

Conversation

olabusayoT
Copy link
Contributor

  • currently we output the line/col info directly from Xerces even if it's -1. We update the code to provide none if line/col <= 0 with test
  • we also fix the doublespace when there is no diagnosticDebugName provided for schema context
  • we looked into implementing a locator, and it was more trouble than it was worth since Xerces doesn't provide it to us

DAFFODIL-2953

@olabusayoT
Only provide valid line/col info from Xerces
22341f4 
- currently we output the line/col info directly from Xerces even if it's -1. We update the code to provide none if line/col <= 0 with test
- we also fix the doublespace when there is no diagnosticDebugName provided for schema context
- we looked into implementing a locator, and it was more trouble than it was worth since Xerces doesn't provide it to us

DAFFODIL-2953
stevedlawrence
stevedlawrence approved these changes Nov 6, 2024
View reviewed changes
Copy link
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

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

+1

daffodil-cli/src/test/scala/org/apache/daffodil/cli/cliTest/TestCLIParsing.scala Outdated
cli.sendLine("123", inputDone = true)
cli.expectErr("[error]")
cli.expectErr(s"Schema context: Location in ${schema.normalize()}")
}(ExitCode.UnableToCreateProcessor)
Copy link
Member

Choose a reason for hiding this comment

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

I imagine we already have a test that tests for failing to resolve schema locations. Maybe we can just update the expected diagnostics to be a bit more specific so we can avoid adding another test?

@olabusayoT
fixup! Only provide valid line/col info from Xerces
0d2e5ac 
- remove additional file/cli test
- update existing tdml test

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

@stevedlawrence stevedlawrence stevedlawrence approved these changes

@mbeckerle mbeckerle Awaiting requested review from mbeckerle

@jadams-tresys jadams-tresys Awaiting requested review from jadams-tresys

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@olabusayoT @stevedlawrence