search

nodeca / argparse

CLI arguments parser for node.js. JS port of python's argparse module.
Other
491 stars 73 forks source link

2.0.0 #112

Closed puzrin closed 4 years ago

puzrin commented 7 years ago

Preliminary requirements:

TODO:

AKST commented 7 years ago

Just a few questions regarding this

Sync with 3.6/3.7 upstream

I can't seem to find much in the 3.7 change log related to argparse, do they have a more detailed change log? I get the impression argparse in python is a solved problem so they're not in a rush to make changes to it.

Improve code maintainability

Other than switching to more usage of ES2016+ features, was there any other things you had in mind for maintainability?

edit: just realised there are make commands for pages + linting + testing

Fix API drift

Which of the following things you mention in drift do you think need changing? Because I think it makes sense to keep camelcasing in Javascript as it's pretty conventional JS. I can possibly see a case for chaining defaultValue to default, however ES2015 name puns & destructuring might suffer as default is a keyword that cannot be used as an identifier, so it'll mean you'll stuff like this

const { default: _default } = argConfig;
# instead of
const { default } = argConfig;

That said I don't see this being something client libraries will likely do often.

I don't think I actually understand "argparse.Const.REMAINDER instead of argparse.REMAINDER" well enough to comment on it.

Move to ES6

Was there any thoughts on how to best to best approach this? Using more ES2016+ features like classes instead of manually setting up prototypes, import/export instead of require/exports.default? Is the plan to go full on babel, or just utilise the ES2016+ features supported by node?

puzrin commented 7 years ago

I can't seem to find much in the 3.7 change log related to argparse, do they have a more detailed change log?

I can not comment this, because don't keep in mind details about implementations. Point is that code should be analyzed to understand details.

Improve code maintainability

Main point is said in first post. File structure is now too far from original. It's not convenient to search appropriate part in python's source. From the other hand, original source is single file of 3000 lines. That's not convenient too.

Need suggestions, what can be done.

Fix API drift

well known differences are documented here https://github.com/nodeca/argparse#argparse. I don't object agains camelCase, but snake_case should be supported too. Also, some rare feature were not ported at all.

Move to ES6

My thoughts are, that es6 is the last thing we should care about. This adds almost nothing, except huge diffs, cluttering tracking of really useful changes. I'd suggest to write new code with es6 features (arrow functions and so on), and leave updating old code to es6 to the last step. That will help to do changes with small steps and keep thing always work.

I see no reasons to use transpilers. Node 4 (or 6) has enougth for this project

AKST commented 7 years ago

My thoughts are, that es6 is the last thing we should care about.

Right I only mention it because you mention it in that list for things for 2.0.0

Point is that code should be analyzed to understand details

It could be worth diffing the module between the releases

Also, some rare feature were not ported at all

Is there a test suite being for pythons argparse where tests could be adapted from?

File structure is now too far from original

Fair enough, I guess that could be helpful in comparing the versions (python v node)

well known differences are documented here https://github.com/nodeca/argparse#argparse.

Mmm, I had a look there and was referring those

I don't object against camelCase, but snake_case should be supported too

I maybe wrong, but I don't think there's an expectation for snake case support. I haven't encounter many libraries that use camelcase in node, except for some fringe libraries. This is coming from someone who prefers python/c style snake case, I just think camelCase is the more appropriate in a javascript library. Especially since a large portion of other libraries do this (including the browser apis and node standard libraries), as well as including the builtin functions & types of JS do this too.

I guess I'm curious as to why this should be supported, like it won't be hard to alias method names on the public API, but stuff like config objects passed into addArgument and constructors as well? That sounds like it could make interpreting the config arguments more error prone and increase maintenance burden of the project.

paulmelnikow commented 6 years ago

Hi, I really like python's argparse and am glad this library exists so I can use it in Node.

Is syncing this with upstream something I could help with? Where would be the easiest place to jump in?

M3TIOR commented 4 years ago

@nodeca Where are your maintainers at on this?

puzrin commented 4 years ago

Rewrite done (in master)