search

tpope / vim-fireplace

fireplace.vim: Clojure REPL support
https://www.vim.org/scripts/script.php?script_id=4978
1.75k stars 139 forks source link

add code formatting via `format-code` #274

Closed christoph-frick closed 7 years ago

tpope commented 7 years ago

Is this standard in cider? It would be a perfect fit for 'formatexpr'.

christoph-frick commented 7 years ago

@tpope yes, see https://github.com/clojure-emacs/cider-nrepl#supplied-nrepl-middleware (wrap-format, format-(code|edn)), I have not found any indicator in the code, that this is optional or only working if the user has clj-fmt installed etc. It's mapped to f b or = in emacs as far as i could find out.

Do you mean, that only formatexpr should be supported or additionally?

tpope commented 7 years ago

Ideally only 'formatexpr'. If we do need a map for whatever reason, we should override gq rather than invent cf.

christoph-frick commented 7 years ago

Changed it to formatexpr. The only thing "lost" by that is, that it now only works for whole lines, yet i have never used it for something else than to format the outermost form or the whole file anyway.

tpope commented 7 years ago

First, some housekeeping: please lowercase the function name for stylistic consistency and pass in arguments, for easier debugging.

function! fireplace#format(lnum, count, char)

When I invoke gqq on the last line of a multiline expression, it hangs indefinitely, presumably because it was fed excess trailing punctuation. Hanging isn't really acceptable. Ideally we'd exclude the irrelevant trailing punctuation from the reformat, but this may be easier said than done.

When I invoke gqap, it deletes the leading newline, jamming it into the preceding function. I'm not sure what you're solving by stripping off all leading newlines, but the logic needs to be smarter.

tpope commented 7 years ago

I just noticed this in my repl:

2017-05-02T17:35:43.224 ERROR clojure.tools.nrepl.server: Unhandled REPL handler exception processing message {:code ...
, :id fireplace-loras-1493326639-38, :ns ..., :op format-code, :session 290c996a-5259-4e81-bdc3-846cd89de5a4}
java.lang.Exception: Unmatched delimiter: ) [at line 1, column 96]

So the hanging is because format-code blows up on invalid input, rather than sending back an error. This needs to be fixed on the CIDER side of things.

christoph-frick commented 7 years ago

Just for the records, It does not hang for me with cider/cider-nrepl "0.14.0" - it just gets silently ignored.

tpope commented 7 years ago

Oh, duh, I should have checked the cider version first.

I would like this to be a lot smarter about about sending whole forms, but I don't think that is a deal breaker for merging. I'm going to do a bit of cleanup and squash to one commit, cool?

christoph-frick commented 7 years ago

sure, go ahead. so for finding the best thing to send, would be finding the largest single form within the bounds?

tpope commented 7 years ago

I'm not 100% sure. I think my first approach would be to start at the trailing punctuation of the last line, and use a searchpairpos() to figure out which one best corresponds to lnum.

tpope commented 7 years ago

Can you tell me why you did all the newline mangling? Looking at making some changes in response to #295 and I'm really tempted to rip it out.

christoph-frick commented 7 years ago

Removing leading empty lines [1] was added because ap on the last form in a file included also the empty line before the form and cljfmt strips it out.

The part substitute part [2] was copied from the existing code.

I am not sure, if cljfmt can be sanely used for "format while you type" (no clue, how it is done in emacs - i assumed "call for motion"). We could check, if the char entered is something /closing/ (like "]})) and then work up to the /opening/.

But until things are solved, would it be better to just have a kill-switch or even an opt-in-switch for setting the formatexpr? My usecase is usually just gqaF and gggqG.

[1] https://github.com/tpope/vim-fireplace/blob/2193122e13fdf9e9af30475d8d9a90746234c1d8/plugin/fireplace.vim#L1611-L1620

[2] https://github.com/tpope/vim-fireplace/blob/2193122e13fdf9e9af30475d8d9a90746234c1d8/plugin/fireplace.vim#L1625

tpope commented 7 years ago

With [cider/cider-nrepl "0.14.0"]:

:PP fireplace#message({'op': 'format-code', 'code': "\n(x)"})
[{'formatted-code': "\n(x)",
  'id': ...,
  'session': ...,
  'status': ['done']}]

What am I missing?

I did kill insert mode formatting from inside the format function. So, no problem there.

christoph-frick commented 7 years ago

So let's get rid of it? Seems to work just fine here.

https://github.com/tpope/vim-fireplace/compare/master...christoph-frick:remove-empty-line-handling-in-format-code?expand=1