Wrong import?

[narimiran]

Line 4 in clCfgInit is:

import std/[streams,parsecfg, os,strutils,tables], cligen/humanUt

This fails on our CIs (for v1.0.x and v1.2.x; strangely, not on devel branch), and on my machine locally with Error: cannot open file: cligen/humanUt.

The fix seems to be a simple removal of the prefix, from cligen/humanUt to just humanUt.

[c-blake]

I cannot reproduce this at all. For me it works on wide range of Nim versions, 0.19.2, 0.20.2, 1.2.0 (initial release), nim-devel either using via nimble or via git clone+add --path to nim.cfg import path methods.

I did test removing the prefix via --path=pkgroot imports (I think I would have to git push to test via nimble). While that also seems to work, I'm not sure killing the qualifier is the right fix here. Is it not more correct namespacing to import a directory-qualified module if conflicts may occur? It's certainly more clear and I named all my submodules under cligen/ not especially unique names under the theory they would always/generally be importable with a qualifying prefix. There's probably some real Nim issue lurking inside here (at least a new diagnostic), but no need to hold our breath figuring that out.

Anyway, I think a cleaner fix here is just remove the import entirely, lifting it into cligen.nim where the include of clCfgInit happens. There is actually a symbol cligen.nim uses from cligen/humanUt anyway and it is bad form to rely on an include file to do a same-level needed import. (I could probably even make cligen.nim:include cligen/clCfgInit an import but then clCfgInit would need to import cligen.nim and I shied away from circular import chain -- perhaps out of a misguided reflex.)

However, since I cannot reproduce your failure in the first place -- not for lack of trying -- I also cannot confirm this is an actual fix. So, I will git push and hopefully you can test against cligen#head on your local system? Please confirm or deny that it's a fix when you can.

[narimiran]

Please confirm or deny that it's a fix when you can.

I can confirm that it now works on my local machine.

(I haven't restarted CIs yet, but I think a new version release is needed before they will pick-up the change.)

[c-blake]

Thanks. Well, I was going to bump to 1.0.0, but I guess I should do 0.9.47 until we get clarity on this. Give me a few minutes.

[c-blake]

Well, I punched 0.9.47. Let me know how the CIs turn out.

Meanwhile I am trying to figure out why some non-cligen package/project cannot override the definition of cligen/clCfgInit anymore. I figure since there is obviously no one-config-file syntax to rule them all that I should leave that open/redefinable. Both I and @kaushalmodi tested this worked just a few weeks ago and now it does not seem to.

[c-blake]

Ok. I got that working. I just forgot pre-breakfast to explicitly include an override between import cligen and dispatch. Seems to work fine.

OTOH, some 3rd party package/project does definitely need to import cligen/humanUt in a path prefix qualified way (at least if it wants to use cligen color facilities as part of its config or any other cligen submodule). So, it is possible that some such would be cligen-default-behavior-redefiners may be troubled by whatever was causing @miran problems. OTOH, maybe being in a distinct package from cligen and doing cligen/humanUt will block whatever the issue is. On the gripping hand combination of both overriding in a 3rd party context and using my colors and actually hitting this not very reproducible bug could be low enough probability to, well, just not worry about. ;-)

[c-blake]

Well, @miran let me know if the CIs turn out ok. I'm optimistic, though. One other observable I had but had not mentioned was that nimsuggest seemed confused by the former way but is fine with the new way.

[Clyybber]

I think you meant to ping @narimiran :)

[narimiran]

I think you meant to ping @narimiran :)

In this case it doesn't really matter because I receive notification because it is the issue I've opened :)

But yeah, if you want to ping me somewhere in the wild, @narimiran is the way to go.

[c-blake]

Oops. Of course, you are right. Sorry to whoever @miran is.

[c-blake]

Also, I figured he'd be notified. I was just using the github name signifier as a "form of address" and re-mentioning it to ensure he knew the "checking-ball" was in his "court". That makes my oops all the more annoying, but maybe makes the real vs user name mixup more understandable.

Hmm. Maybe we need a compiler to check github comments. ;-) In other words, @Clyybber is The Human Compiler! ;-)

[narimiran]

Well, @narimiran let me know if the CIs turn out ok. I'm optimistic, though.

Your optimism was well-founded. cligen is now green on our CIs.

Thanks for a prompt fix!

[c-blake]

You're welcome. Glad it worked. Strange I couldn't reproduce.