version 0.6.3 breaks puppet-lint head

[mschuchard]

I can no longer use puppet-lint head with linter-puppet-lint 0.6.3. linter-puppet-lint no longer outputs any warnings to the display. It worked as expected in 0.6.2 with puppet-lint head.

[Arcanemagus]

Are you getting a warning about using an older version of puppet-lint? The only functional change between those versions was #51.

[mschuchard]

I am not getting a warning about an older version. I should also mention, so that you don't have to assume it, that I did flip back to puppet-lint 1.1.0 official and it was working.

[Arcanemagus]

Does the HEAD version of puppet-lint change the output? If the output lines don't match this regex then nothing will be parsable.

[mschuchard]

Looks like warning is now WARNING in the upcoming puppet-lint. Should be a simple change to future-proof this Linter. Can you guys do a quick fix or should I PR?

I have no idea about error because I use my own linter for those and puppet-lint misses the vast majority of errors.

[mschuchard]

I believe I found the culprit

However, unclear why this was working with 0.6.2. There must be something more going on here.

[mschuchard]

Any word on the plan for updating the regexp to const regexLine = /^(warning|WARNING|error|ERROR):?\s(.*)\s(\d+)\s(\d+)$/;

[Arcanemagus]

You could also just specify the i flag, with a comment noting that different versions have different casing so it is necessary.

If you want to put up a PR and test this I can merge it unless somebody in @AtomLinter/linter-puppet-lint has a different idea?

[mschuchard]

Excuse me: I added the : and it should be :?. I updated the comment above with the fix.

Typical puppet-lint out put now looks like:

ERROR: foo::bar not in autoload module layout on line 1
WARNING: double quoted string containing no variables on line 5

which shows that it no longer seems to always display the column which means current/future puppet-lint would be identified as an 'old version' by this linter, although the notification of this seems to be failing.

[mschuchard]

Ok actually the regex is working fine and I just forgot the --log-format arguments. I just tried reverting the main.js to the 0.6.2 version and that still does not work with puppet-lint head so the breaking change happened somewhere else.

At this point I have no idea how to get this package working with the next version of puppet-lint.

[Arcanemagus]

The simplest plan that I can think of is to just release a major version update of this package when this new version of puppet-lint that completely changes the output is finally released.

This new version would only parse the output of the new version, with possibly a warning if output from the older version is found.

[mschuchard]

I don't think this has to do with the output. I think something insidious is happening.

I am going to do some more testing intermittently for some time. If I find something interesting, or end up fixing it myself, then I will report back.

[mschuchard]

I figured it out. It was so obvious that I feel dumb, but then again there was no output to work off of.

I had the 'skip 80 char check' enabled and HEAD has the current style guideline of '140 char.' The invalid flag was being passed to puppet-lint and the 'invalid flag' error was being passed back to stdout, albeit with exit code 1, and not stderr so it was never picked up on the linter-puppet-lint stack trace.

Unfortunately puppet-lint is basically dead, otherwise I would go PR that the invalid flag output is logged to stderr and not stdout so that linter-puppet-lint would pick up the error and stack trace instead of it being silently output to stdout. My personal solution will be to just edit the code for linter-puppet-lint to have the '140 char skip' and then reload the editor so atom picks up the changes and linter-puppet-lint works with puppet-lint head. I already tested that this works.

Another solution may be to try to catch the exit code 1 from puppet-lint like in my recent question at steelbrain/atom-linter#128, but I have been trying to pull that off unsuccessfully and nobody at atom.io's forums seems to know how to do what steelbrain is describing either.

Thanks for the effort.

[Arcanemagus]

The full example for what he's describing would be:

helpers.exec(atom.config.get('linterExecutablePath'), args, {stream: 'stdout'}).then(output => {
  // Do stuff
}).catch(error => {
  // Do error stuff
});

But that wouldn't help in this case since puppet-lint isn't putting anything out on stderr so exec() isn't throwing an error to catch since it doesn't look like the return code is handled there, only the contents of stdout and stderr.

[mschuchard]

Yeah that was what I was trying to do, but I was hoping Javascript could catch/except/rescue precisely so that hopefully it could catch PuppetLint::InvalidFlag, or something like that, even without stderr.

I was probably being over optimistic.

[Arcanemagus]

Javascript doesn't know anything about the run of puppet-lint other than what it output on stdout/stderr, getting debugging information like that cross-executable is a far more difficult task than simply running it 😉.