-
Notifications
You must be signed in to change notification settings - Fork 113
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: Add untilTag option to readFile #134
base: master
Are you sure you want to change the base?
Conversation
@pieper suggests switching it to allow namified version instead and I agree. I'll do that before merging |
@swederik May be helpful also to have an while (!bufferStream.end()) {
if (breakOnNextLoop === true) {
break;
}
const readInfo = DicomMessage.readTag(bufferStream, syntax);
const cleanTagString = readInfo.tag.toCleanString();
if (untilTag && untilTag === cleanTagString) {
break;
}
if (stopAfterTag && stopAfterTag === cleanTagString) {
breakOnNextLoop = true;
}
...
} Would submit pull request but looks like you want to allow namified version to these options. Happy to help if you like, but still getting my bearings with this library! |
Opened PR #149 to address these issues, including namified support. Would appreciate feedback/review! |
@jmhmd Great PR on #149, I think the @pieper @swederik @jmhmd
The only major deviation will be that this will return a dict where a tag is present with null values and null value representation. I don't think this should be a big issue since the existence of the tag key is a useful flag for whether the |
True, but as a matter of style I'd like to encourage the use of the keywords from the data dictionary and keep the hex out of the code for readability.
That make sense. There's a general guideline to use positive statements for boolean flags, so maybe better is |
Test it with: