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.

Sign up for GitHub

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

Jump to bottom

Restart the idris compiler after every commmand if it was killed #54

Merged
merged 5 commits into from
Sep 18, 2015
Merged

Restart the idris compiler after every commmand if it was killed #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e8b0e89
restart the idris compiler after every commmand if it was killed
archaeron Aug 29, 2015
a0175f8
ability to stop the compiler with a command
archaeron Aug 29, 2015
11f2edf
less dire error message if the user stopped the compiler manually
archaeron Sep 16, 2015
efbce99
updated the changelog for #54
archaeron Sep 18, 2015
c8d9b8e
Merge branch 'master' into issue-51
archaeron Sep 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Added

- Restart the idris compiler after every commmand if it was killed [#54](https://github.com/idris-hackers/atom-language-idris/pull/54)
- added the ability to style all the idris-panels

### Fixed
Expand Down
4 changes: 4 additions & 0 deletions lib/idris-controller.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class IdrisController
'language-idris:proof-search': @runCommand @doProofSearch
'language-idris:typecheck': @runCommand @typecheckFile
'language-idris:print-definition': @runCommand @printDefinition
'language-idris:stop-compiler': @stopCompiler

isIdrisFile: (uri) ->
uri?.match? /\.idr$/
Expand All @@ -46,6 +47,9 @@ class IdrisController
@messages.attach()
@messages.hide()

stopCompiler: =>
@model?.stop()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this trigger one of the process alerts to fire, or does it die without a fight?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It triggers the message: It looks like the compiler was closed or crashed. What do you think? It could be good to know that it worked. Could be distracting on the other hand though. But having to kill the compiler is distracting anyway probably :D

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current alerts are very dire. To confirm a successful user action, we would want it not to appear like something went wrong.

Do we report the process state in the status bar? (I'd check directly, but on my phone just now.) Being able to inspect that and see that it is now dead might be all the confirmation needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some info in the statusbar, but I'm thinking of removing it, because I have the feeling that there is already a bit too much information there.
The new version shows if a file has been loaded in a small pane below the code. Maybe it's better to unify all the idris messages in such a pane.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the message less dire. It looks like this now if you stop the compiler manually:
stop_compiler


runCommand:
(command) =>
(args) =>
Expand Down
36 changes: 21 additions & 15 deletions lib/idris-ide-mode.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ class IdrisIdeMode extends EventEmitter
buffer: ''
idrisBuffers: 0

constructor: ->
pathToIdris = atom.config.get("language-idris.pathToIdris")
@process = spawn pathToIdris, ['--ide-mode']
@process.on 'error', @error
@process.on 'exit', @exited
@process.on 'close', @exited
@process.on 'disconnect', @exited
start: ->
if (not @process?) || @process.killed
pathToIdris = atom.config.get("language-idris.pathToIdris")
@process = spawn pathToIdris, ['--ide-mode']
@process.on 'error', @error
@process.on 'exit', @exited
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the exit-event may or may not fire after an error has occurred. If you are listening on both events to fire a function, remember to guard against calling your function twice. (https://nodejs.org/api/child_process.html#child_process_event_error)

Do we test for "did we already bug them about an error?" in exited()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, good point. Not entirely sure what would be the most elegant way to do this.
Maybe converting the callbacks to RxJS observables and using something like this: https://github.com/Reactive-Extensions/RxJS/blob/master/doc/api/core/operators/debounce.md

I don't like the callbacks anyway

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your feelings about moving from CoffeeScript to Idris for generating the JS?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to, but my Idris is not good enough to do this at the moment :)

@process.on 'close', @exited
@process.on 'disconnect', @exited
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like, in place of the connected property that wasn't working for you, you're watching for the process to die via all the various events?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connected property is a bool that should tell if it's connected right now (at least according to the docs). I use killed now to check if the compiler is running, and only start a new instance if it's not.
The events are used if the compiler crashes or is killed by the user.


@process.stdout.setEncoding('utf8').on 'data', @stdout
@process.stdout.setEncoding('utf8').on 'data', @stdout

send: (cmd) ->
Logger.logOutgoingCommand cmd
Expand All @@ -38,13 +39,18 @@ class IdrisIdeMode extends EventEmitter
atom.notifications.addError e.short, detail: e.long

exited: (code, signal) ->
short = "The idris compiler was closed or crashed"
long =
if signal
"It was closed with the signal: #{signal}"
else
"It (probably) crashed with the error code: #{code}"
atom.notifications.addError short, detail: long
if signal == "SIGTERM"
short = "The idris compiler was closed"
long = "You stopped the compiler"
atom.notifications.addInfo short, detail: long
else
short = "The idris compiler was closed or crashed"
long =
if signal
"It was closed with the signal: #{signal}"
else
"It (probably) crashed with the error code: #{code}"
atom.notifications.addError short, detail: long

running: ->
!!@process
Expand Down
1 change: 1 addition & 0 deletions lib/idris-model.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class IdrisModel
if !@ideModeRef
@ideModeRef = new IdrisIdeMode
@ideModeRef.on 'message', @handleCommand
@ideModeRef.start()
@ideModeRef

stop: ->
Expand Down