-
Notifications
You must be signed in to change notification settings - Fork 49
Add a debug trace hook, and also some enhancements to the fhirpath parser to read position information. #174
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
base: master
Are you sure you want to change the base?
Conversation
…hile parsing (and correct the text for some node types) Introduce a debugger function to the options, and call that during the evaluation of each node
…ure/BP-Debugger
This is a PR into the HAPI fhirpath engine to compare to if it's of any interest. |
…ure/BP-Debugger
… and also clean up the text handling, to only read it when it's required. Process the quantity and external contant delimited text handling better between the parser and engine to remove parser complexity from the engine. No longer requires terminalNodeText.
This is now working really well. |
let parent = parentStack.pop(); | ||
// special case handling for functions that use the argument as a type, not a value | ||
// need to read out the text for them | ||
// Note: doesn't handle the delimited text cases correctly in the engine later on |
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.
Delete? Haven't found a case where there is a problem.
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.
Didn't we discover this at the connectathon?
(and decided to leave it in as a comment and address as a seperate issue - existing problem discovered rather than something I introduced)
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.
There is no handling the delimiter processing for the identifier
type which can be of IDENTIFIER
or DELIMITEDIDENTIFIER
The correct place to do this is in the parser, where it knows which type it is, and thus handles it.
The current code just removes any preceding or training ` chars using a regex replace.
Not an issue for FHIR as no types have special processing required and thus need the delimiting.
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.
This is the tracker that reports this issue for addressing after this is completed
#178
(and fixing this will also remove the regex cost in parsing too - unless the delimitedidentifier is processed, use of IDENTIFIER won't need to do that processing)
…ine uses the `text` property (which is set correctly below)
Sorry for building this on top of the other PR (with the children bug fixes...)
Debugging Enhancements:
src/fhirpath.js
: Added a debugging hook (ctx.debugger
) toengine.doEvalSync
, allowing custom debugging callbacks to inspect evaluation results. Also added support for passing adebugger
option inapplyParsedPath
to enable debugging. [1] [2]Parsing Enhancements:
src/parser/index.js
: Added position information (line, column, and length) for various node types during parsing to improve traceability and debugging. [1] [2]Haven't done any unit tests yet, but have verified the changes using them in the fhirpath lab to work exactly how I was after.
Wanted to ensure this style of fix would be ok before continuing.
And I'll be doing more testing across various expressions with the lab to ensure there are no other issues.