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

formatting ns forms #13

Closed
expez opened this issue Feb 1, 2015 · 14 comments
Closed

formatting ns forms #13

expez opened this issue Feb 1, 2015 · 14 comments

Comments

@expez
Copy link

expez commented Feb 1, 2015

For refactor-nrepl I made the terrible mistake of trying to write my own pretty-printing code to output the newly-cleaned ns form. This started out as just a few lines of code which worked in 95% of cases, but has now grown to a a huge mess which badly needs to be rewritten, or preferably thrown out altogether.

Will it ever be in the scope of cljfmt to format an ns form nicely?

@expez expez changed the title pretty printing ns forms formatting ns forms Feb 1, 2015
@weavejester
Copy link
Owner

I don't see why not.

cljfmt can already correct the indentation of an ns form, but doesn't currently order the namespaces or insert linebreaks when necessary. However, both those things would come under formatting in my book.

@expez
Copy link
Author

expez commented Feb 2, 2015

Glad to hear you consider this in-scope for cljfmt.

Here's what the clean-ns op in clj-refactor does:

  • Eliminate :use clauses
  • Sort required libraries, imports and vectors of referred symbols
  • Rewrite to favor prefix form, e.g. [clojure [string test]] instead of two separate libspecs
  • Raise errors if any inconsistencies are found (e.g. a libspec with more than one alias.
  • Remove any unused namespaces, referred symbols or imported classes.
  • Remove any duplication in the :require and :import form.

In the interest of not duplication any code I researched the following solutions when I needed to format the ns. Some of this might interest to you:

clojure.core/pprint Has a small amount of special handling for pretty-printing the ns form, but not enough. The relevant code is here

fipp which is a pretty-printer construction kit didn't work out of the box, but I now regret trying to hack my own code instead of relying on this lib.

slamhound has its code for pretty-printing the ns form here.

The rewriting to use prefix form drove some of these issues I had. I find the prefix form easier to read, but it's a pain to manage manually (which is why I think it isn't used more widely). I'd like libspecs that aren't vectors to be printed on their own line(s), but breaking at line 80:

(:require [clojure data edn xml
           [instant :as inst]
           [pprint :refer [pprint]]])

I'll likely write an op for clj-refactor to call out to cljfmt once it's a bit more mature.

@weavejester
Copy link
Owner

I'm not sure how much of that is in scope for cljfmt. Currently it just affects whitespace, and doesn't change the meaning of the source code. If cljfmt were to affect more, I think it would have to be controlled by an optional flag.

@expez
Copy link
Author

expez commented Feb 4, 2015

Perhaps I was unclear: I just want cljfmt to format the ns nicely, the rest clj-refactor can handle (like eliminating duplication and rewriting to use prefix form). I'm hoping that if just dump the entire ns form on a single line cljfmt will insert newlines in it in a sensible way.

@weavejester
Copy link
Owner

Oh I see. Yes, that's perfectly within scope of cljfmt.

@bbatsov
Copy link
Contributor

bbatsov commented Feb 8, 2015

I'll likely write an op for clj-refactor to call out to cljfmt once it's a bit more mature.

Actually we might add this directly to CIDER (I can imaged commands like format-buffer and format-region (or rather format-form). clj-rewrite deps shouldn't be problematic for most people.

@expez
Copy link
Author

expez commented Feb 8, 2015

👍

@bbatsov
Copy link
Contributor

bbatsov commented Feb 8, 2015

And now we have cider commands for cljfmt. :-) See clojure-emacs/cider@b661d10

The initial support is pretty basic, but will cover the needs of most people.

@expez
Copy link
Author

expez commented Feb 8, 2015

Very cool!

@xpe
Copy link

xpe commented Apr 6, 2015

@expez Is your issue still open? Or has it been resolved?

@expez
Copy link
Author

expez commented Apr 6, 2015

@xpe this issue is still open

I've written some refactorings for moving files and directories of clj files for clj-refactor. Right now I parse and update the ns form of all affected namespaces but then I have to use emacs and clojure-mode to visit the files afterwards to get right indentation of the ns form.

Needless to say I'd be quite happy to skip that last step. Not only because it makes it easier but because the op becomes slow when a large amount of files are affected.

@expez
Copy link
Author

expez commented May 18, 2015

@weavejester is this likely to get solved? I realize this isn't the most sexy problem to spend limited time on :/

@weavejester
Copy link
Owner

When someone submits a pull request for it.

It's not something I personally need, and isn't a bug but a new feature, so it's fairly far down my list of priorities.

@expez
Copy link
Author

expez commented Jul 31, 2022

Hi, I see this was closed in a commit that mentions ns sorting. Has the formatting / pretty-printing (proper indentation, breaking of long lines in sensible places etc) been solved previously?

Many thanks for your tireless and consistent maintenance of you open source projects @weavejester 🙇

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

No branches or pull requests

4 participants