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

Fix case for "clCfg" in Release Notes in the instructions for breaking changes [v0.9.28] #105

Closed
kaushalmodi opened this issue May 21, 2019 · 8 comments

Comments

@kaushalmodi
Copy link
Contributor

Hello,

I was a bit confused when the instructions for fixing the backward-incompatible changes in the Release Notes didn't work: https://github.com/c-blake/cligen/blob/master/RELEASE-NOTES.md#version-0928

Then looking into the tests, I realized that I need to do clCfg.version, and not ClCfg.version.

btw thanks for continually maintaining and updating this package!

@c-blake
Copy link
Owner

c-blake commented May 21, 2019

Hey. Sorry to hear about the confusion. I almost sent you a specific update recommendation, too (I sent one to #103 (comment)).

I can see the confusion in that I say TYPE.FIELD for the general but INSTANCE.FIELD for the specific, but I do say INSTANCE.FIELD in the two examples on the right hand side right after the general. I realize capital and lowercase C are very similar in most fonts, though "cl" for "command-line" is an ancient and well known abbreviation (most often used as the start of CLI).

I wasn't sure how to emphasize that you don't have to use the convenience global var clCfg from the module. You can declare/define your own either from scratch or using clCfg as a template, and pass it via the new macro parameter cf. So, maybe you're saying I should use lowercase 'C' throughout that table part, and then explain you don't have to use the global? (btw, you're welcome. :-)

(edit: markdown ate my bracketed tokens.)

@c-blake
Copy link
Owner

c-blake commented May 21, 2019

Also, linked in that #103 (comment) you see a new versionFromNimble that lets you just have just one place were you edit the version string - the nimble file. Unless you're using some git hash version string instead of an easier to understand numeric-ish one.

@kaushalmodi
Copy link
Contributor Author

kaushalmodi commented May 21, 2019

You can declare/define your own either from scratch or using clCfg as a template, and pass it via the new macro parameter cf

Understood, I read your notes in hurry.

Unless you're using some git hash version string instead of an easier to understand numeric-ish one.

I use git describe --tags HEAD that's the best of both worlds (1: readable numeric version, and 2: git hash). That git command returns just the tag name if the HEAD is tagged. If the HEAD has newer commits after the last tag, it returns a string containing the last tag + the git hash.

@c-blake
Copy link
Owner

c-blake commented May 21, 2019

Ah. Fine point re: best of both worlds version strings. I updated the example in testVersion.nim to use that. Also takes Unix head out of the example, making the example more portable.

I'm open to re-wording suggestions on the RELEASE_NOTES, but if you don't have anything, we can maybe close the issue. No rush, though. It's hard to explain some things and confusion is in some ways a fleeting and precious resource. :-)

@kaushalmodi
Copy link
Contributor Author

heh, isn't that crazy?

image

@c-blake
Copy link
Owner

c-blake commented May 21, 2019

Yeah. You & I are pretty in sync in the literal sense! I was thinking that as I pressed enter and saw the update.

@c-blake
Copy link
Owner

c-blake commented May 21, 2019

Also, I'm unsure if a "like/emoji" use on a thread is sufficient to get you and @pb-cdunn notified about the just-closed very long standing issue #2 / #30. So, I'll mention it here in case you're interested. Probably best to reply on one of those threads if you reply.

I think 3-ish years ago when it first surfaced one could get the doc comments for fields out of the AST to avoid duplication in the new xhelp, but I think they may no longer be present. Perhaps nim core would support them again by popular demand? Some attempt was already made nim-lang/Nim#8516, but I believe not well received.

Also, if anyone has any thoughts of how I can avoid the 'x' in xhelp and friends, I am all ears.

@c-blake
Copy link
Owner

c-blake commented May 22, 2019

Never mind about the 'x' in xhelp bit. I figured out an internally ugly but externally fine workaround.

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