Add support for Deno shortcuts and wildcards #508
Conversation
Signed-off-by: Luka Leer <luka.leer@gmail.com>
Signed-off-by: Luka Leer <luka.leer@gmail.com>
| import { CommandInfo } from '../command'; | ||
| import { CommandParser } from './command-parser'; | ||
|
|
||
| const OMISSION = /\(!([^)]+)\)/; |
There was a problem hiding this comment.
Would you mind adding a comment explaining what this Regexp does?
It isn't clear its intent unless you read the rest of the code and, even then, it's a bit confused.
Thanks in advance.
There was a problem hiding this comment.
Yeah, I was confused when I read it at first as well. Good catch, just added a comment. Do you think that clarifies it enough?
There was a problem hiding this comment.
Thank you.
I was just wondering why we'd need to look for ) in the script command?!
I guess most of the people (myself included) don't know we could use something like this deno task lint:*(!fix) as a valid command.
Perhaps, adding an example in the comment would help as well.
There was a problem hiding this comment.
This behaviour is currently already documented, though I do see how it could be difficult to find for someone less familiar with the tool. I do think this is a bit out of scope for this pull request, though, so perhaps it would be better to tackle this separately?
Signed-off-by: Luka Leer <luka.leer@gmail.com>
| commandInfo.command.match(/^(node|npm|yarn|pnpm|bun):(\S+)(.*)/) || []; | ||
| if (!cmdName) { | ||
| const [, prefix, script, args] = | ||
| /^(npm|yarn|pnpm|bun|node|deno):(\S+)(.*)/.exec(commandInfo.command) || []; |
There was a problem hiding this comment.
Out of curiosity, was there any advantage of changing from string.match to regexp.exec() here?
AFAICT both are generally equivalent, but maybe I'm missing something.
No dramas if it's just your preference :)
There was a problem hiding this comment.
Mostly personal preference, yeah. From what I've heard, String.match() and RegExp.exec() function identically to each other if the global flag is not set, in fact, String.match() uses RegExp.exec() under the hood, so using the latter carries a very minor performance benefit with it.
With the release of Deno 2, I figured I'd have a look if the amazing shortcut and wildcard features of this tool supported it. Sadly, they didn't. Luckily, adding it wasn't too difficult. One thing to note is that Deno uses its own
deno.jsonfile with what it calls 'tasks', but can also fall back on Node'sscriptsdirective in thepackage.json.For a cleaner reviewing process, I recommend looking at the individual commits, as it doesn't quite seem to understand I renamed some files.