-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adding some tags to the tags feature #84
Conversation
@@ -118,18 +132,65 @@ dataExtractor = | |||
.tags >> Set.toList >> Encode.list Encode.string | |||
|
|||
|
|||
declarationVisitor : Node Declaration -> ModuleContext -> ( List never, ModuleContext ) | |||
declarationVisitor node ({ tags } as context) = | |||
moduleDefinitionVisitor : Node Module -> ModuleContext -> ( List never, ModuleContext ) |
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.
What are all the List Never
for?
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.
It's kind of required, because I only output empty lists. But looking it from the other way, it signals from the type signature that those list must be empty, since you cannot create a generic type out of nowhere (I called it never
, but it could have been anything, void
is another nice one).
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.
I take it the elm review api requires you return the List something
? never seems like a good choice for the name, it matches the Never type.
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.
At first I read your sentence as "it never seems like a good choice for the name" hahaha :D
I think I tried using Never
before, but I vaguely remember it didn't work? I'll try again and clarify.
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.
Nice work!
Part of #83
It's been a while :)
I had a bit of time, so I went through the list of missing tags and added a couple, some very basic ones, some a bit more fun.