-
Notifications
You must be signed in to change notification settings - Fork 34
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
Only use current fix examples for tests #633 #665
Conversation
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.
MetafixBindTest.java
: I couldn't find anything that needed changing, except for the@MetafixToDo
s. What do you have in mind?syntax-sample.fix
: This one is tricky. It's used specifically to test the Vim plugin, which already has support for some syntax that isn't implemented yet. I don't think we should bother with it too much. As soon as the Metafix grammar is rewritten, I'll see to it that we get boolean operators :)
Yes, the TODOs which are "won't fix"s since the morph collectors and other morph concepts are not reused in Fix anymore. Should we just delete these tests? @blackwinter
Fine with me |
Is that so? When was it decided?
You should probably ask @fsteeg, since he's the one who introduced them (in f328a13). If, in fact, we no longer plan to implement those, we should definitely delete the tests IMO. |
Yes, makes sense, these are probably leftovers from the initial, morph-based approach to implementing fix. |
Another thing that I spotted is that the java tests still have |
I stand by my earlier statement:
I don't see why it should be addressed here at all. It still is and will be valid syntax. |
See: #633
Not sure how to adjust the file:
https://github.com/metafacture/metafacture-core/blob/9ed5d4e8844998f899031861646a8384fc191c15/metafix/src/test/java/org/metafacture/metafix/MetafixBindTest.java
Also here is already an example of boolean operators for conditionals that are not supported yet:
https://github.com/metafacture/metafacture-core/blame/39ce013e731250b20698b61455733fadd80ae651/misc/vim/syntax/syntax-sample.fix#L15-L24 See #610