Skip to content
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

Show errors for ambigious symbol #51

Closed
alaviss opened this issue Nov 24, 2018 · 5 comments
Closed

Show errors for ambigious symbol #51

alaviss opened this issue Nov 24, 2018 · 5 comments

Comments

@alaviss
Copy link

alaviss commented Nov 24, 2018

Example:

import cligen, options

func get() = discard

dispatch get

This will fail due to ambiguous symbol get provided by both options and the current module, and the error message is certainly unhelpful:

nimble/pkgs/cligen-0.9.17/cligen.nim(215) dispatchGen
test.nim(5, 10) template/generic instantiation of `dispatch` from here
nimble/pkgs/cligen-0.9.17/cligen.nim(215, 19) Error: node is not a symbol
@c-blake
Copy link
Owner

c-blake commented Nov 24, 2018

First, I agree it's not a great error message for a common situation. However, the error message in question is not from me, but from Nim. It has recently changed to Error: node lacks field: symbol. It's also compile time messaging/erroring out not some run time exception I could catch. So, I'm not sure I have any control at all over what it says.

@alaviss
Copy link
Author

alaviss commented Nov 24, 2018

You can check the node type beforehand. If it's nnk{Closed,Open}SymChoice (it was nnkClosedSymChoice in this case), then you can just print out every overload found with a nice error message.

A lazy approach is to use typed{nkSym} as the parameter to offload the job to the compiler. The error message would not be nice, but it's understandable.

@c-blake
Copy link
Owner

c-blake commented Nov 24, 2018

Ah. Good suggestions. Do you know a way at the macro call site to refer (more verbosely, obviously) to a specific get overload? If so, I could also put that in the error message as guidance.

@c-blake
Copy link
Owner

c-blake commented Nov 24, 2018

(I mean, of course, the type signature not just the module qualifier...)

c-blake pushed a commit that referenced this issue Nov 24, 2018
#52 as well as adding a
new `dispatchName` feature for `dispatchGen` (which makes more
sense users may be calling generated dispatchers themselves more)
and retiring some old-versions-of-Nim compatibility `when`
clauses and a few other clean-ups like subcoommand -> subcommand.

Add add a test program for dispatch(a simple qualified symbol) as
test/QualifiedSym.nim and a more thorough test/QualifiedMulti.nim
that tests both `cmdName`-only and `cmdName`-`dispatchName` sets.
@c-blake
Copy link
Owner

c-blake commented Nov 24, 2018

I went with the lazier typed{nkSym} approach. I agree it's an ugly error message, but I feel it's a better overall solution for you to submit an issue to Nim to get them to format it more nicely for everyone. See test/QualifiedSym.nim.

@c-blake c-blake closed this as completed Nov 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants