Skip to content
This repository has been archived by the owner on Jan 28, 2019. It is now read-only.

Refactoring defprotcol for syntax consistency with defn #155

Merged
merged 2 commits into from
Oct 18, 2016

Conversation

arrdem
Copy link
Collaborator

@arrdem arrdem commented Oct 13, 2016

Same vein as #146, the more syntax a user has to remember the more awkward it is. I didn't realize for instance that protocol methods could be variadic, or that protocols could have docstrings attached. If they can have docstrings, why don't they have metadata same as other def forms?

Why is the parsing all done inline in one ungodly block?

Shavin' yaks....

@arrdem arrdem force-pushed the feature/defprotocol-consistency branch from 1ae0355 to ac11175 Compare October 13, 2016 21:14
@arrdem
Copy link
Collaborator Author

arrdem commented Oct 14, 2016

Before:
screenshot 2016-10-13 23 14 20

After:
screenshot 2016-10-13 23 14 01

I also took the opportunity to implement some better (but still not spec-equivalent) error message in the defprotocol implementation.

The downside here is that this is a very breaking change with regards to existing library tooling. While it codegens and runs just fine, the Jaunt test suite is now hosed because tools.namespace isn't valid anymore.

@arrdem
Copy link
Collaborator Author

arrdem commented Oct 16, 2016

So after thinking about this for a while - while it's nice that individual protocol methods now get leading docstring? attr-map? same as everything else it's also a huge breaking change. Either the parser should be non-positional (which technically the original parser was) or docstring? attr-map? should be trailing arguments after arglists so that this remains an additive change.

@arrdem
Copy link
Collaborator Author

arrdem commented Oct 18, 2016

Opting for trailing - (method-name arglists* docstring? attr-map?).

defprotocol itself now accepts leading docstring and attr-map. Its
method forms now accept a trailing docstring and attr-map.
@arrdem arrdem force-pushed the feature/defprotocol-consistency branch from 335f0d9 to e75a532 Compare October 18, 2016 18:02
@arrdem arrdem merged commit 1c60d24 into develop Oct 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant