This changes `monroe-eval-buffer' to preserve line numbers.

[technomancy]

Note that this does change the semantics of C-c C-k because it will load the file from disk, so unsaved changes will not be reflected. IMO it's much more important to have line numbers; if you really need to eval unsaved changes you can still use C-c C-r.

In order to avoid being surprised by this change, we now prompt to save the current buffer if it's unsaved.

[sanel]

Thanks! I see couple of problems here:

• It replicates monroe-load-file functionality.
• It changes current behavior - you'll no longer be able to eval buffer without saving it, which I'm using extensively. Evaluating region would require another set of key combos to select the whole buffer.
• It adds another entry for direct talk to nrepl protocol. I'm planning to add socket repl support at some point and this will make it challenging

I'd introduce global var that will retain old behavior (I'm a bit pedantic regarding consistent behavior), e.g. monroe-save-on-eval-buffer and do something like this:

(defun monroe-eval-buffer ()
  "Evaluate the buffer."
  (interactive)
  (if monroe-save-on-eval-buffer
    (let ((this-buffer buffer-file-name))
       ;; (buffer-modified-p) check is called inside (save-some-buffers)
       (save-some-buffers nil (lambda () (equal buffer-file-name this-buffer)))
       (monroe-load-file this-buffer))
    (monroe-eval-region (point-min) (point-max)))

[sanel]

Also another problematic use case with saving buffer: this will no longer work on remote connections and I use this feature too...

[technomancy]

• It changes current behavior - you'll no longer be able to eval buffer without saving it, which I'm using extensively. Evaluating region would require another set of key combos to select the whole buffer.

This is true, but the existing function is simply equivalent to C-x h C-c C-r, so it seemed redundant to me.

To be honest I am surprised anyone was using it; programming without line numbers is extremely difficult, especially when Clojure already has a reputation for unhelpful stack traces it seems unwise to encourage practices that make it worse.

• It adds another entry for direct talk to nrepl protocol. I'm planning to add socket repl support at some point and this will make it challenging

It actually doesn't--the monroe-get-stacktrace was already making a call over the nrepl protocol when monroe-detail-stacktraces is set.

I noticed that a very common problem when using monroe was to use C-c C-k with a file that failed to compile, and if the error was near the top of the file, it would scroll off the screen and I would not have a way to know that there was a problem. Failing silently is a pretty serious usability problem and a common source of errors in my experience.

Anyway, isn't socket-repl support already covered by existing packages like https://github.com/clojure-emacs/inf-clojure? If you set the subprocess command to netcat it should do what you want.

I'd introduce global var that will retain old behavior (I'm a bit pedantic regarding consistent behavior)

Rather than introducing a new var, maybe it's better to fix the monroe-load-file command to prompt for unsaved buffers and add it to the readme. I didn't know about it since the readme only points to monroe-eval-buffer which is basically unusable for me due to the line numbers issue.

Also another problematic use case with saving buffer: this will no longer work on remote connections and I use this feature too...

Actually because it calls monroe-translate-path-function it works fine with remote buffers once you configure it. This should probably be mentioned in the readme though.

[technomancy]

I noticed monroe-load-file prompts every time for a path, making it more tedious to use as a replacement for monroe-eval-buffer. What do you think about making it prompt when there is a prefix arg and using the current buffer otherwise?

[technomancy]

Actually hold off on this one; I realized monroe-translate-path-function needs to be bidirectional.

[technomancy]

The nrepl protocol's eval op actually claims to support passing in file and line arguments which would mean we wouldn't have to switch to monroe-load-file in order to get working stack traces; unfortunately these fields are actually ignored in practice; seems to be a bug somewhere server-side in the nrepl stack.

[sanel]

It actually doesn't--the monroe-get-stacktrace was already making a call over the nrepl protocol when monroe-detail-stacktraces is set.

Sorry, mixed up patches :)

Actually because it calls monroe-translate-path-function it works fine with remote buffers once you configure it. This should probably be mentioned in the readme though.

Agree, but this will add another option to make it workable (although another elisp variable does the same). How you envisioned monroe-translate-path-function to be used?

[bbatsov]

The nrepl protocol's eval op actually claims to support passing in file and line arguments which would mean we wouldn't have to switch to monroe-load-file in order to get working stack traces; unfortunately these fields are actually ignored in practice; seems to be a bug somewhere server-side in the nrepl stack.

Yeah, we added this for CIDER a while ago. This made it possible to set the proper location metadata with pretty much every client eval command.

I can assure that this works and if it's ignored for you likely you're running an old version of nREPL.

[technomancy]

Going to go ahead and close this out and open another which clarifies the difference between monroe-load-file and monroe-eval-buffer since it is the opposite of what nrepl.el and slime used to do.