technomancy / slamhound

Slamhound rips your namespace form apart and reconstructs it.
Other
473 stars 38 forks source link

Better formatting of the namespace declaration #15

Closed tomfaulhaber closed 12 years ago

tomfaulhaber commented 12 years ago

This brings to mind http://bit.ly/yg4mUO

I think that this patch captures all the variations, but there are a lot of them, so let me know if any output looks wrong. Or if something is simply not to taste.

I will make a similar patch to clojure.pprint so that this will be built into code-dispatch in 1.4 (don't worry, it won't break this version).

Here is what my output looks like for an absurdly complex namespace:

(ns autodoc.build-html                                                                                     
  "This is the namespace that builds the HTML pages themselves.                                            
It is implemented with a number of custom enlive templates."                                               
  {:skip-wiki true, :author "Tom Faulhaber"}                                                               
  (:refer-clojure :exclude [empty complement])                                                             
  (:import [java.util.jar JarFile]                                                                         
           [java.io File FileWriter BufferedWriter StringReader                                            
                    BufferedInputStream BufferedOutputStream                                               
                    ByteArrayOutputStream FileReader FileInputStream]                                      
           [java.util.regex Pattern])                                                                      
  (:require [clojure.string :as str])                                                                      
  (:use [net.cgrand.enlive-html :exclude (deftemplate)]                                                    
        [clojure.java.io :only (as-file file writer)]                                                      
        [clojure.java.shell :only (sh)]                                                                    
        [clojure.pprint :only (pprint cl-format pprint-ident                                               
                               pprint-logical-block set-pprint-dispatch                                    
                               get-pretty-writer fresh-line)]                                              
        [clojure.data.json :only (pprint-json)]                                                            
        [autodoc.collect-info :only (contrib-info)]                                                        
        [autodoc.params :only (params expand-classpath)])                                                  
  (:use clojure.set clojure.java.io clojure.data clojure.java.browse                                       
        clojure.inspector clojure.zip clojure.stacktrace))
technomancy commented 12 years ago

I'd be nervous about maintaining such a huge block of code in a single function, but it sounds like you're saying this belongs in clojure.pprint and that it's only included in slamhound for convenience for those of us that aren't using a new enough version of Clojure (one that happens to not exist yet)?

tomfaulhaber commented 12 years ago

Yeah, that's what I'm saying. After we let it settle in slamhound and don't see any cases that I missed, I'll put it into code-dispatch and push it for 1.4.

The function decomposition leaves something to be desired. I tend to write big functions for the pprint dispatch cause pprint can blow the stack pretty easily. This is probably an exception though, since ns should always be at top level and the amount of structure underneath it is usually pretty limited. No matter how you chop it up, though, formatting the ns directive intuitively is pretty nasty.

In any case, I'm happy to support it. The language of pprint dispatch is, indeed, an obscure one.

technomancy commented 12 years ago

Works for me. Thanks for implementing it!

I'll try it out for a while and take a shot at #13 before cutting the next stable slamhound release.

jsnikeris commented 12 years ago

Nice job, Tom! I've been working on this for a couple weeks, and although my output is acceptable, I like yours much better!

This was a hairy problem. Good work!

tomfaulhaber commented 12 years ago

Hi Joe,

Thanks for the feedback.

I've made a patch at http://dev.clojure.org/jira/browse/CLJ-963 that's slated to be added in Clojure 1.5. But there are more structures that might do with some special casing (protocols come to mind). If this is something you wanted to play around with, I would happily review a patch (just make sure you have the CA thing done!).

Enjoy,

Tom

On 01/15/2012 08:53 AM, Joe Snikeris wrote:

Nice job, Tom! I've been working on this for a couple weeks, and although my output is acceptable, I like yours much better!

This was a hairy problem. Good work!


Reply to this email directly or view it on GitHub: https://github.com/technomancy/slamhound/pull/15#issuecomment-3500103