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

add public method afterSpawnProcess to Linter class #556

Closed
wants to merge 667 commits into from
Closed

add public method afterSpawnProcess to Linter class #556

wants to merge 667 commits into from

Conversation

harrytruong
Copy link

Gives subclasses a chance to read or change the child
process, after creating new BufferedProcess while lintFile.

Useful for passing data via stdin (process.process.stdin).

This is a backward compatible improvement.

dmnd and others added 30 commits September 14, 2014 11:52
Test Plan:

Looked at a lint message. Edited the message in the inspector to make it really
long, to see what happened for linebreaking. It wraps eventually so this seems
good enough.
In `deactivate`, `remove` is called, but before this change, `remove` doesn't
exist. The real fix for this is probably to make `InlineView` a real `View`,
but for now this will stop errors.

The fact that Atom crashes is a separate issue that I'll make a report for.

Test Plan:
Press `ctrl-cmd-opt l` and Atom doesn't crash
Fixes #212.

Test Plan:
Linted a file and saw no errors
When there are no cursors, this throws. Though in that case it looks like both
these views can just return early since they have nothing to render.

Test Plan:
Crossed fingers
Test Plan:
Ran specs, they all pass
Fix positioning when inline view is on last line
Attempting to prevent the following exception:

    Uncaught Error: No screen line exists when converting buffer row to screen row /Applications/Atom.app/Contents/Resources/app/src/display-buffer.js:932
      module.exports.DisplayBuffer.screenPositionForBufferPosition /Applications/Atom.app/Contents/Resources/app/src/display-buffer.js:932
      module.exports.Marker.getHeadScreenPosition /Applications/Atom.app/Contents/Resources/app/src/marker.js:211
      Marker /Applications/Atom.app/Contents/Resources/app/src/marker.js:45
      module.exports.DisplayBuffer.getMarker /Applications/Atom.app/Contents/Resources/app/src/display-buffer.js:1124
      module.exports.DisplayBuffer.markBufferRange /Applications/Atom.app/Contents/Resources/app/src/display-buffer.js:1156
      module.exports.Editor.markBufferRange /Applications/Atom.app/Contents/Resources/app/src/editor.js:1186
      LinterView.display linter-view.coffee:180
      LinterView.processMessage linter-view.coffee:159
      (anonymous function) linter-view.coffee:1
      (anonymous function) linter-view.coffee:142
      Linter.processMessage linter.coffee:150
      (anonymous function) linter.coffee:127
      BufferedProcess.triggerExitCallback /Applications/Atom.app/Contents/Resources/app/src/buffered-process.js:52
      (anonymous function) /Applications/Atom.app/Contents/Resources/app/src/buffered-process.js:59
      (anonymous function) /Applications/Atom.app/Contents/Resources/app/src/buffered-process.js:105
      EventEmitter.emit events.js:129
      (anonymous function) net.js:461

Test Plan:

Can't repro that issue, but did test that error messages still work.
Fixes #218.

Test Plan:
Opened a java file and didn't get ENOTEMPTY errors.
Added a check to return early for the common case where messages haven't
changed, and the cursor is on the same line.

Test Plan:
Moved a cursor around on the same line and it feels a bit faster.
Add LESS to list of available linters
Fix issue links in changelog
Fixes #165.

Test plan:

 * Opened a file
 * Caused a lint error on the final line
 * Deleted a line from the middle of the file
 * No error
@harrytruong
Copy link
Author

This PR was submitted as part of a fix to AtomLinter/linter-jshint#25. Subclasses like linter-jshint need access to stdin on the spawned child class.

I'm hoping @iam4x can review this PR, and the related linter-jshint issue.

And sorry for failing the CI build. It's such a lightweight PR, I don't know where it went wrong...

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

Successfully merging this pull request may close these issues.