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

Investigate an easy way to execute commandline tools #18

Closed
orta opened this issue Oct 27, 2016 · 6 comments
Closed

Investigate an easy way to execute commandline tools #18

orta opened this issue Oct 27, 2016 · 6 comments
Labels
enhancement You Can Do This This idea is well spec'd and ready for a PR

Comments

@orta
Copy link
Member

orta commented Oct 27, 2016

I'll want something that takes flow & ESLint's JSON output and allows us to show errors for that. In ruby this is as simple as

`echo "OK"`

I don't know what that should look like in JS, perhaps we'll need something like?

danger.run("flow check --json", output: "json", (result: any) => {
  if (!result.passed) {
    fail("Boo! Better fix them types")
  }
})

Given the prevalence of JSON output, we can optionally handle the transition from text for users too

@orta
Copy link
Member Author

orta commented Oct 27, 2016

This will NO-OP when running inside hosted Danger

@orta
Copy link
Member Author

orta commented Dec 23, 2016

OK, I"ve one back to this. I've found a good idea to base it on, VS Code's problem matchers

It's support an arg like:

{
    // The problem is owned by the cpp language service.
    "owner": "cpp",
    // The file name for reported problems is relative to the opened folder.
    "fileLocation": ["relative", "${workspaceRoot}"],
    // The actual pattern to match problems in the output.
    "pattern": {
        // The regular expression. Example to match: helloWorld.c:5:3: warning: implicit declaration of function ‘prinft’ [-Wimplicit-function-declaration]
        "regexp": "^(.*):(\\d+):(\\d+):\\s+(warning|error):\\s+(.*)$",
        // The first match group matches the file name which is relative.
        "file": 1,
        // The second match group matches the line on which the problem occurred.
        "line": 2,
        // The third match group matches the column at which the problem occurred.
        "column": 3,
        // The fourth match group matches the problem's severity. Can be ignored. Then all problems are captured as errors.
        "severity": 4,
        // The fifth match group matches the message.
        "message": 5
    }
}
danger.run({
  command: "eslint",
  errorPatterns: [
        {
            regexp: "^([^\\s].*)$",
            file: 1
        },
        {
            regexp: "^\\s+(\\d+):(\\d+)\\s+(error|warning|info)\\s+(.*)\\s\\s+(.*)$",
            line: 1,
            column: 2,
            severity: 3,
            message: 4,
            code: 5,
            loop: true
        }
    ]
})

We may be able to take the code directly from vscode, don't think they separate it into modules much so it'd need a license attaching and bringing inline but it seems like the right abstraction.

@orta
Copy link
Member Author

orta commented Dec 23, 2016

Just like VS Code, we can ship with some common JS ones, looking through extensions and vscode for examples.

@orta
Copy link
Member Author

orta commented Jan 8, 2017

I've got roughly half way with integrating the VS Code matcher runner into DangerJS. It's 1400 LOC though, and our TSConfig is considerably more strict than the one on VS Code.

So I think I may need to do a clean room re-implementation instead.

@macklinu macklinu mentioned this issue Mar 21, 2017
@orta
Copy link
Member Author

orta commented Mar 21, 2017

Ah yeah, this is two things.

  1. an easy way to run a command
  2. conforming to a standard for generating errors and warnings

  1. probably don't need. In this repo I did this:
        const output = child_process.execSync(`yarn why jest --json`)

        // Comes as a series of little JSON messages
        const usefulJSONContents = output.toString().split(`{"type":"activityEnd","data":{"id":0}}`).pop() as string
        const asJSON = usefulJSONContents.split("}\n{").join("},{")

which basically does what I wanted.

  1. Still could be useful, but the file in vscode is non-trivial to port. It'd probably end up working better by just parsing JSON form those tools instead.

so, I guess this could probably be closed.

@orta
Copy link
Member Author

orta commented Sep 21, 2018

I'm not to sold on the need for this now, you can use any node module - so let people do that.

@orta orta closed this as completed Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement You Can Do This This idea is well spec'd and ready for a PR
Projects
None yet
Development

No branches or pull requests

1 participant