Keep fire from getting out of control, minor changes

[nickdrozd]

This package is hilarious. I actually came across it when a picture was posted in a comment thread for a completely non-technical article along with "What a burn!". So congratulations, fireplace has hit the mainstream!

Anyway, the main change in this PR is to allow killing the *fireplace* buffer directly, without running fireplace-off. Previously, killing the buffer would lead an unkillable buffer getting created.

There are also some small code changes, like whitespace cleanup.

Now, time to get cozy...

https://www.youtube.com/watch?v=xJ_qbUwzyLo

[coveralls]

Coverage decreased (-1.2%) to 89.024% when pulling 92a9ecdb3462d9e4fdaf411a8f3fc355a27eff9c on nickdrozd:changes into 23a444f749bcb2b804593e3b2cb9c73cc59231fb on johanvts:master.

[coveralls]

Coverage decreased (-1.2%) to 89.024% when pulling 92a9ecdb3462d9e4fdaf411a8f3fc355a27eff9c on nickdrozd:changes into 23a444f749bcb2b804593e3b2cb9c73cc59231fb on johanvts:master.

[coveralls]

Coverage remained the same at 90.244% when pulling 5044adefae1fcc9430938ad551443465cd9ea48d on nickdrozd:changes into 0e166d7ac59e606f3197c6b3be4467a5f978201c on johanvts:master.

[johanvts]

Thank you, sounds like a nice change however it seems to be bugged for me. Having run you version of fireplace.el, when I kill the fireplace with C-x-k and then try to start a new fireplace I get a "Buffer is read only: #<buffer fireplace>" error.

[nickdrozd]

Rebased to fix merge conflict

[nickdrozd]

@johanvts I had that problem too. I think it has to do with the old and new versions conflicting. You might try restarting Emacs with the changed code only, and also make sure that any .elc files are deleted. You could also try starting Emacs with -Q and then loading the changed Fireplace code.

I say all this because I'm pretty sure that it works fine locally, both with and without -Q. But of course, "works locally!" is notoriously unreliable, so maybe there's something I'm missing :)

Running GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30) of 2018-11-30, if the version matters.

[nickdrozd]

I added a test that I think convincingly demonstrates that the bug is fixed. It can be run with the shell command found in the test file. The test case fails before the major change and passes, so I think it works. (The shell command could be made into a proper makefile, or I'm sure it could be done with another test runner.)

[johanvts]

I have been starting with "emacs -Q" and loading the changed file, I get the read only error as described. I'm using emacs 25 on Windows GNU Emacs 25.3.1 (x86_64-w64-mingw32) of 2017-09-12 Where can I see your test?

[nickdrozd]

I put in commit Add failing test-fire-raging-out-of-control before any commits that make substantial code changes (Github isn't showing the commit order correctly). The test is in emacs-fireplace-tests.el.

(ert-deftest test-fire-raging-out-of-control ()
  (fireplace)
  (let ((buf (get-buffer "*fireplace*")))
    (should buf)
    (kill-buffer buf))
  (sit-for 2)
  (let ((buf (get-buffer "*fireplace*")))
    (should-not buf)))

It runs fireplace, kills the *fireplace* buffer, waits, then verifies that there is no buffer with that name.

I run the test with emacs -Q --batch --directory ./ --directory ./test --load test/emacs-fireplace-test.el --funcall ert-run-tests-batch-and-exit.

That test fails before the change and passes after. Actually, that test command should pass on every commit, because the :expected-result :failed is added, but that flag is removed in the main commit Keep fire from getting out of control.

As I said, the test increases my confidence that the fix is correct, but if it isn't, I'd like to know why. And this package is awesome, so I also want this bug fixed :)

[ebpa]

I just tested the changes against a semi-recent emacs master and 25.2.2 (using emacs -Q). It kills fine for me with both q and M-x kill-buffer (no errors or orphaned timers). 5044ade also works fine with the rest of my dotemacs. I had not previously installed emacs-fireplace on this machine.

Let me know if you want to to check anything in particular.

[johanvts]

Tried it again, maybe I tested code from the wrong commit before, anyway it does indeed work. Fireplace buffers can be killed and new fires made without the read only error. Thank you.