-
-
Notifications
You must be signed in to change notification settings - Fork 253
Add support for Deno shortcuts and wildcards #508
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
gustavohenke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great addition, thank you!
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.