c-blake / cligen

Nim library to infer/generate command-line-interfaces / option / argument parsing; Docs at
https://c-blake.github.io/cligen/
ISC License
507 stars 24 forks source link

fix(macUt): ignore comments in nimble file #229

Closed pysan3 closed 11 months ago

pysan3 commented 11 months ago

Hi, thanks for the awesome library!

In order to use release-please, I need to add a comment on the version = line in xxx.nimble. Example: https://github.com/pysan3/minorg/blob/main/minorg.nimble

Currently, the fromNimble function does not take that into account, and the function returns what is not intended. i.e. file.fromNimble("version") -> 2.0.1" # {x-release-please-version

This PR fixes the problem by cutting the comment before returning, as well as some minor fixes to the function.

Hope this helps ;)

c-blake commented 11 months ago

Well, if it were me writing the code, I'd probably use a string slice { e.g. line[0..<comment].split('=') instead of line.substr(0, comment - 1).split('=') }, but Nim Is Choice™ and this is also fine as-is. :-) So, I'll just merge it. Thanks!

pysan3 commented 11 months ago

Thanks, I'm actually fairly new to this community so good to know that ;)

if it were me writing the code, I'd probably use a string slice

Just out of curiosity, is it cuz it's shorter?

c-blake commented 11 months ago

Well, it is shorter, but also I like that ..< operator more than the - 1 for both iterators/for-loops and slices. Admittedly, what makes these "off by 1" errors adjustments more|less obvious to a reader is quite subjective, though.

pysan3 commented 11 months ago

makes these "off by 1" errors more obvious

That's true. Thanks, I'll choose slices from now on lol

c-blake commented 11 months ago

https://github.com/c-blake/bu/blob/main/doc/nrel.md may also be of interest to you. (It can do things like update all the versions of packages which are depended upon.) { Really bu has so many little atoms in it, I usually say "5..10 are likely of interest". }

pysan3 commented 11 months ago

Ah, nice. Yes, we need some automation tool to make everything working lol.

This repo is impressive. I gotta read them thru to get more knowledge on nim. ig it's like the best resource I found online haha (literary can't find much info other than the official docs)

pysan3 commented 11 months ago

Would you mind releasing a new tag 1.6.16 with this change? @c-blake

c-blake commented 11 months ago

Done. I guess I did it too fast and didn't update the RELEASE-NOTES.md file, but eh. Pretty non-interacting changes, TBH.

pysan3 commented 11 months ago

Thanks <3