search

bblfsh / sdk

Babelfish driver SDK
GNU General Public License v3.0
23 stars 27 forks source link

Add Drop operation to Fields #369

Closed dennwc closed 5 years ago

dennwc commented 5 years ago

As requested in https://github.com/bblfsh/cpp-driver/pull/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 for Optional.

Signed-off-by: Denys Smirnov denys@sourced.tech

dennwc commented 5 years ago

OK, I see that the comment is confusing. Any ideas on how to make it more clear?

dennwc commented 5 years ago

Ready for another review round.

juanjux commented 5 years ago

More nitpicking trying to look at it from the perspective or a new, clueless and confused DSL user (because I've been there haha).

juanjux commented 5 years ago

Perfection has been reached 👌

bzz commented 5 years ago

Regarding the discussion in https://github.com/bblfsh/sdk/pull/369#discussion_r259029902 - would you be so kind to help me understand the expected behavior of - {Name: "specifiers", Drop: true, Op: Arr()}, is that suppose to match&drop only empty array?

Because from what I can tell - right now this results in check: unused field(s) on node ImportDeclaration: specifiers 😕

juanjux commented 5 years ago

Could it be the case that there is a specifiers field that is not an empty array (so the Drop wouldn't match) and it's not being handled?

bzz commented 5 years ago

It could, but I wanted to verify the expectations here: {Name: "specifiers", Drop: true, Op: Arr()} is supposed to match&drop specifiers only in case it's an empty array - would that be correct?

juanjux commented 5 years ago

I think it is but less wait for the DSL guru @dennwc to confirm this.

dennwc commented 5 years ago

Yes, it's correct. If it's not an empty array the whole operation will be reverted.