Skip to content

feat(markdown-docx): add conditional transformer - #397#453

Merged
algomaster99 merged 1 commit intoalgoo-ooxmlfrom
k-kumar-01/i397/conditional-transformer
Aug 18, 2021
Merged

feat(markdown-docx): add conditional transformer - #397#453
algomaster99 merged 1 commit intoalgoo-ooxmlfrom
k-kumar-01/i397/conditional-transformer

Conversation

@K-Kumar-01
Copy link
Copy Markdown
Collaborator

Signed-off-by: k-kumar-01 kushalkumargupta4@gmail.com

Add the transformer OOXML<->CiceroMark for conditional.

Changes

  • transformation logic: OOXML<->CiceroMark
  • rules: OPTIONAL_RULE
  • tests

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to master from fork:branchname

transformation logic: OOXML<->CiceroMark
rules: OPTIONAL_RULE
tests

Signed-off-by: k-kumar-01 <kushalkumargupta4@gmail.com>
@K-Kumar-01
Copy link
Copy Markdown
Collaborator Author

@algomaster99
Requesting your review.

Comment on lines +406 to +414
/**
* for optional hasSome and whenSome stand for same as would be in the ciceromark.
* for conditional, the following relations with optional can be made
* optional | conditional
* hasSome | isTrue
* whenSome | whenTrue
* whenFalse | whenFalse
* The properties for conditional are not added therefore.
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Helps a lot. Thanks!

Copy link
Copy Markdown
Contributor

@algomaster99 algomaster99 left a comment

Choose a reason for hiding this comment

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

Looks good to me as the changes look more or less similar to Optional node.

@dselman One question - what is the semantic difference between Conditonal and Optional? They look like they exist for the same purpose.

@algomaster99
Copy link
Copy Markdown
Contributor

@K-Kumar-01 Ready for merge?

@K-Kumar-01
Copy link
Copy Markdown
Collaborator Author

@algomaster99
Yes this one can be merged.
Didn't have many changes except the keys.
I provided an explanation regarding that in the comments itself.

@algomaster99 algomaster99 merged commit 58551c9 into algoo-ooxml Aug 18, 2021
@algomaster99 algomaster99 deleted the k-kumar-01/i397/conditional-transformer branch August 18, 2021 15:19
@K-Kumar-01 K-Kumar-01 added this to the Implement conditonal milestone Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants