Closed milesj closed 4 years ago
@milesj I'm happy to work on ES2015, Jest, and the build pipeline. I could probably help with typings, but have more experience with Flow than Typescript. Please let me know if anything is already in progress, or if there is anything in particular I should start on.
@Nathan-Schwartz Sounds great!
I'm currently working on the Babel/ESLint portion of the code, so that will be good to go soon. I also just merged #182 which includes some ES2015 goodness. Once I have this build pipeline merged, I'll divvy up the ES2015 tasks.
Let's put off Jest/Flow until the very end, as they are quite involved changes.
@Nathan-Schwartz The build tools have been updated.
I will tackle the big files for ES2015 conversion, mainly command-instance
, command
, ui
, util
, and vorpal
. Feel free to tackle the other files.
If you do, please do 1 file per PR, and be sure that npm run lint
validates for that file, and npm test
still completes.
Lastly, the Babel config supports stage-2, so feel free to use class properties and object destructuring.
@milesj Awesome, will do. I hope to get a start on it tonight
@milesj awesome!
A few points:
I would like to drop Babel entirely. This is a front-end tool. This means it is seldom going into legacy projects, it mainly caters to new projects. There isn't a ton of need for legacy support, so we should bump the support to just the latest versions of Node and skip the transpile step.
I intend Vorpal to be as slim as possible, comparable to commander. It's been bloated due to Babel and Inquirer. The only reason Inquirer is huge is that it uses Lodash. Just something to think with if you have any ideas.
See bash-parser. @parro-it has been doing great work on this project. I want Vorpal to support most common bash usage out of the box, giving an instant cross-platform bash feel to any Node CLI. This has been the main goal for Vorpal 2.0.
@dthree Since Node.js 6.5+ supports 100% ES5 syntax, not using Babel should be easy. We'd have to use require
over import
, but that isn't too bad.
Unfortunately, I don't believe there is object spread support, but there is Object.assign
.
Exactly. I think keeping require
will be find and definitely worth it.
@dthree @Nathan-Schwartz I removed Babel from the pipeline and updated the minimum Node.js requirement to 6.10 (the oldest LTS). Let's see how this works going forward.
Awesome!
I guess this is abandoned?
@shaharmor Not really. I've just been too busy the past few months to work on it.
Recommend TypeScript, have had great experience with it in my 18 months of using it. Not sure about Flow, but...TS is pretty hawt. If you deprecate autocompletion, is there going to be something in its stead? that's one of the features that I would actually like to see improved. See: https://github.com/dthree/vorpal/issues/285
Is this thread still open to contribution? If so, I'd like to pop my head in the door and offer my time to port this project to TypeScript, if that's still on the table?
It would be a reasonable undertaking, but not a hugely complicated one. The toughest step would be to clear out the current PRs to allow a change of this size to go through without too much collateral!
I've started to use Oclif, which is written in TS. https://github.com/oclif/oclif
Part of the reason why I haven't contributed much.
@iainreid820 Totally open to converting the 2.0 branch from Flow to TS. It's pretty easy, as I've done it on a few projects.
TypeScript support? Why not write the library in TypeScript? Ditch Babel and use TS, imo.
Last year I wrote PR to experiment with the idea of introduce bash-parser usage in the 2.0 version.
It could be done, and it should resolve many of the issues regarding argument parsing and file/pipe redirections, but it emerge an uncopatibility with the two packages: bash-parser parse command names containing spaces according to POSIX standard, while vorpaljs allow for such commands to be called without quoting.
So, if you register a command with name "say hello" actually vorpaljs allow to call it without quotes, while if we introduce bash-parser, it will not anymore, because it parse it as "say" command and an "hello" argument.
This was made evident from a bunch of specifics test that was failing in that PR, and it will require a decision if we want to introduce such a breaking change.
If that was the case, the 2.0 release is probably the moment to introduce it. @dthree any comments/idea ?
@ORESoftware Oclif is written in TS.
@parro-it It would be in the 2.0 release, which is ok, as breaking changes are allowed. My 2 cents.
@milesj cool, yeah the first post said something about TS support and throughout there is talk of babel build pipeline. maybe update the very first post, idk.
@milesj The 2.0 branch hasn't been updated in a year and doesn't seem to contain any typescript source. While the master branch continues to be updated and there's discussion on #158 about definitions in progress for using v1 in Typescript projects, but nothing committed yet to DefinitelyTyped.
Just wondering where to start with using Vorpal in a Typescript project. Is a Typescript version coming, or should I copy the community provided definition file for now and hold out for official @types/vorpal
support.
@timkinnane The discussion is about using TS for 2.0, not that Vorpal has been written in it. No one is currently working on it.
@milesj - you said above...
It is written in TS.
Thanks for the clarification anyway.
@timkinnane That was in reference to Oclif.
@milesj yeah that was really confusing tbh, we were clearly talking about vorpal not oclif wrt TS support
Yeah sorry about that. I updated the other post to reflect that.
Last year I wrote PR to experiment with the idea of introduce bash-parser usage in the 2.0 version.
It could be done, and it should resolve many of the issues regarding argument parsing and file/pipe redirections, but it emerge an uncopatibility with the two packages: bash-parser parse command names containing spaces according to POSIX standard, while vorpaljs allow for such commands to be called without quoting.
So, if you register a command with name "say hello" actually vorpaljs allow to call it without quotes, while if we introduce bash-parser, it will not anymore, because it parse it as "say" command and an "hello" argument.
This was made evident from a bunch of specifics test that was failing in that PR, and it will require a decision if we want to introduce such a breaking change.
If that was the case, the 2.0 release is probably the moment to introduce it. @dthree any comments/idea ?
I just saw this post, and i'm not sure this is a good idea to break this feature. (which I extensively use I admit)
But I think there should be a way to use bash parser and interpret the ast by reworking the command matching. Maybe I could have a look to your branch and try to implement this, what do you think @parro-it?
(@parro-it I start working on your branch, checking it out, but I didn't found any code bash-parser. Seems to be missing some commit?)
@AdrieanKhisbe no, there no missing commits, I stopped working on the branch as soon as I discover that a breaking change is needed...
Anyway, a first step in order to do what you are purposing should be to implement the required changes on bash-parser. A possible way to do it could be to inject known commands in bash-parser, forcing it to parse them even if they contains spaces...
Just thinking, I'm not sure if it doable for real...
@parro-it Okey dokey. An alternative wouldn't be to parse the command registered, to have a tree, and then to compare it with the actual command? (and see if both the "command", and the leadings "arguments" match
Yes, that should be simpler, and could be a post-process operation. Good idea!
Hi @milesj , I believe there's a TypeScript PR at #328 with a Issue #313 #158 and we can review & merge it first, how do you think about it?
I'd like to join for help and I'm ready to start collaborating!
I'm not working on this anymore (I went and wrote my own CLI https://milesj.gitbook.io/boost/cli). I'll close this thread as it's confusing.
Now that I'm an active collaborator, I would like to get a very high level roadmap going as a way to ensure progress.
v2.0
autocompletion
(IN PROGRESS)@dthree for visibility.