Add Drop operation to Fields #369
Conversation
|
OK, I see that the comment is confusing. Any ideas on how to make it more clear? |
Signed-off-by: Denys Smirnov <denys@sourced.tech>
|
Ready for another review round. |
uast/transformer/ops.go
Outdated
| // To handle nil fields, see Opt operation. | ||
| Optional string | ||
| Op Op // operation used to check/construct the field value | ||
| // Drop the field if it exists. Op should be set and it will be called to check the |
There was a problem hiding this comment.
I would add:
- Optional is implied (even if its implicitly told by "if it exists").
uast/transformer/ops.go
Outdated
| // To handle nil fields, see Opt operation. | ||
| Optional string | ||
| Op Op // operation used to check/construct the field value | ||
| // Drop the field if it exists. Op should be set and it will be called to check the | ||
| // value before dropping it. If the check fails, the whole transform will be canceled. |
There was a problem hiding this comment.
Maybe "the whole transform will be cancelled like if you used Any()" which is also didactic.
| // Drop the field if it exists. Op should be set and it will be called to check the | ||
| // value before dropping it. If the check fails, the whole transform will be canceled. | ||
| // | ||
| // Please note that you should avoid dropping fields with Any unless there is no |
There was a problem hiding this comment.
"you should avoid dropping fields with Any (explicit or implicit) unless there is no..."
There was a problem hiding this comment.
See the comment thread above: #369 (comment)
I remove the implicit Any.
|
More nitpicking trying to look at it from the perspective or a new, clueless and confused DSL user (because I've been there haha). |
Signed-off-by: Denys Smirnov <denys@sourced.tech>
|
Perfection has been reached 👌 |
|
Regarding the discussion in #369 (comment) - would you be so kind to help me understand the expected behavior of - Because from what I can tell - right now this results in |
|
Could it be the case that there is a |
|
It could, but I wanted to verify the expectations here: |
|
I think it is but less wait for the DSL guru @dennwc to confirm this. |
|
Yes, it's correct. If it's not an empty array the whole operation will be reverted. |
As requested in bblfsh/cpp-driver#26, add an option to drop a field. It will work the same way as
Optional: "x", Op: Any(), except that it won't create a variable forOptional.Signed-off-by: Denys Smirnov denys@sourced.tech