8000 Proper __ne__ comparisons by Vlad-Shcherbina · Pull Request #6 · docopt/docopt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Proper __ne__ comparisons #6

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 14, 2012
Merged

Proper __ne__ comparisons #6

merged 2 commits into from
Apr 14, 2012

Conversation

Vlad-Shcherbina
Copy link
Contributor

(it's more a test of pull request function than actual contribution)

Unlike python 3, in python 2.* non-equality comparison is not derived from equality one, thus the patch.

@Vlad-Shcherbina
Add __ne__ comparisons; minor changes.
0d53726 
Namely, implement dict-based comparison for `Options` and change list comprehensions to generator expressions.
@Vlad-Shcherbina
Check type in Options comparison.
b065fd2
@keleshev
Copy link
Member

I didn't know about that:

Unlike python 3, in python 2.* non-equality comparison is not derived from equality one

but the only reason I added __eq__ was in order to be able to test more easily (see test_docopt.py), so I don't know if __ne__ is needed at this point of time.

Also thanks for pointing out that I can save some characters with using generator expression.

Not sure what is the advantage of comparing __dict__s instead of __repr__s in this case, though.

I pull, but I don't know if any of these changes will still be there after I merge the "pattern-matching" branch (https://github.com/halst/docopt/tree/pattern-matching), because it is changing a lot.

Anyway, do you have any cool ideas of where this project could get going? I'm implementing pattern-matching for usage-messages, but ideas are always welcome.

keleshev pushed a commit that referenced this pull request Apr 14, 2012
Merge pull request #6 from Vlad-Shcherbina/master
d557ce0 
Proper __ne__ comparisons
@keleshev keleshev merged commit d557ce0 into docopt:master Apr 14, 2012
@Vlad-Shcherbina
Copy link
Contributor Author

Speaking of ideas, I'm not sure. But let me criticize what you are doing now :)

It seems you intend to support arbitrary nesting of () and [] in usage strings (like '[--sort [--reverse]]'). Regex-based parsing is not the best approach to this problem, and I suggest using recursive descent parser instead. Perhaps I'll find some time soon and implement it by myself.

Also, your parse results currently reflect syntactic structure as opposed to the logical one. I mean, you have Parens(a, VerticalBar, b) where something like Choice(a, b) would be more appropriate.

@keleshev
Copy link
Member

If you look more closely, I don't use regex to parse usage, I use it for tokenization. parse() is a recursive descent parser.

Also I agree that it's best to reflect semantic structure, and things like VerticalBar are just a stub. I also used to have Ellipsis, having Argument('N'), Ellipsis stand for N ..., but now I parse it into OneOrMore(Argument('N')).

I used names like Parens and Brackets because I can't yet figure out good names for them ;-). I think Choice is quite good for (smth | smth | smth).

@keleshev
Copy link
Member

Also, if you want to add bigger contributions, I suggest we first discuss stuff (before writing code).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Vlad-Shcherbina @keleshev
0