Better formatting of the namespace declaration #15

Merged
merged 2 commits into from Jan 8, 2012

Projects

None yet

3 participants

Contributor

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))
Owner

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)?

Contributor

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.

Owner

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.

@technomancy technomancy merged commit f9159a1 into technomancy:master Jan 8, 2012
Contributor

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!

Contributor

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:
#15 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment