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

2.0 Roadmap #234

Closed
3 of 11 tasks
milesj opened this issue May 25, 2017 · 32 comments
Closed
3 of 11 tasks

2.0 Roadmap #234

milesj opened this issue May 25, 2017 · 32 comments

Comments

@milesj
Copy link
Contributor

milesj commented May 25, 2017

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

  • Audit and fix/close issues
  • Audit and update dependencies
  • Add minor features discovered during issue audit
  • Deprecate autocompletion (IN PROGRESS)
  • Switch to Yarn
  • Update build pipeline (babel, eslint, etc)
  • Update to 100% ES2015 (IN PROGRESS)
  • Async commands
  • Sub-commands
  • Switch to Jest???
  • Add Flow/TypeScript support??? (IN PROGRESS)

@dthree for visibility.

@milesj milesj changed the title 1.5 and 2.0 Roadmap 2.0 Roadmap May 25, 2017
@Nathan-Schwartz
Copy link
Contributor

@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.

@milesj
Copy link
Contributor Author

milesj commented May 26, 2017

@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.

@milesj
Copy link
Contributor Author

milesj commented May 26, 2017

@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.

@Nathan-Schwartz
Copy link
Contributor

@milesj Awesome, will do. I hope to get a start on it tonight

@dthree
Copy link
Owner

dthree commented May 26, 2017

@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.

@milesj
Copy link
Contributor Author

milesj commented May 26, 2017

@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.

@dthree
Copy link
Owner

dthree commented May 26, 2017

Exactly. I think keeping require will be find and definitely worth it.

@milesj
Copy link
Contributor Author

milesj commented May 27, 2017

@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.

@dthree
Copy link
Owner

dthree commented May 29, 2017

Awesome!

@shaharmor
Copy link

I guess this is abandoned?

@milesj
Copy link
Contributor Author

milesj commented Oct 17, 2017

@shaharmor Not really. I've just been too busy the past few months to work on it.

@ORESoftware
Copy link
Contributor

ORESoftware commented Nov 6, 2017

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: #285

@iainjreid
Copy link

iainjreid commented Jun 6, 2018

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!

@milesj
Copy link
Contributor Author

milesj commented Jun 6, 2018

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.

@milesj
Copy link
Contributor Author

milesj commented Jun 6, 2018

@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.

@ORESoftware
Copy link
Contributor

ORESoftware commented Jun 6, 2018

TypeScript support? Why not write the library in TypeScript? Ditch Babel and use TS, imo.

@parro-it
Copy link

parro-it commented Jun 6, 2018

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 ?

@milesj
Copy link
Contributor Author

milesj commented Jun 6, 2018

@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.

@ORESoftware
Copy link
Contributor

@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.

@timkinnane
Copy link

@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.

@milesj
Copy link
Contributor Author

milesj commented Jul 6, 2018

@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.

@timkinnane
Copy link

timkinnane commented Jul 6, 2018

@milesj - you said above...

It is written in TS.

Thanks for the clarification anyway.

@milesj
Copy link
Contributor Author

milesj commented Jul 6, 2018

@timkinnane That was in reference to Oclif.

@ORESoftware
Copy link
Contributor

@milesj yeah that was really confusing tbh, we were clearly talking about vorpal not oclif wrt TS support

@milesj
Copy link
Contributor Author

milesj commented Jul 6, 2018

Yeah sorry about that. I updated the other post to reflect that.

@AdrieanKhisbe
Copy link
Contributor

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?

@AdrieanKhisbe
Copy link
Contributor

(@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?)

@parro-it
Copy link

parro-it commented Oct 9, 2018

@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...

@AdrieanKhisbe
Copy link
Contributor

@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

@parro-it
Copy link

Yes, that should be simpler, and could be a post-process operation. Good idea!

@huan
Copy link

huan commented Jul 2, 2020

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!

@milesj
Copy link
Contributor Author

milesj commented Jul 3, 2020

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.

@milesj milesj closed this as completed Jul 3, 2020
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

10 participants