Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Support code that is all indented #59

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

ForbesLindesay commented Jun 30, 2012

This removes the excess indentation. For example the output in code of the following:

            /**
             * Test indented function
             */
            function indented() {
                foo();
            }

Used to be:

function indented() {
                foo();
            }

Which is clearly wrong, with this change, it would be outputted as:

function indented() {
    foo();
}

With an extra property of codeIndent which would let you add the spaces back in if you wanted to for some reason (it represents the number of spaces used for the line with the minimum number).

It leaves tabs in tact and all tests still pass.

Contributor

ForbesLindesay commented Jul 2, 2012

I really need this fix in my project, could you let me know whether you plan to accept the pull request, since if you don't, I'll just have to start maintaining my own separate fork.

Owner

tj commented Jul 3, 2012

sounds like a good thing to fix to me, but we should be able to get away with something like:

var spaces = str.match(/^\n?( *)/)[1].length;
var re = new RegExp('^ {' + spaces + '}', 'gm');
str = str.replace(re, '');
Contributor

ForbesLindesay commented Jul 3, 2012

There's a problem with that in that the following is completely valid

if (isFoo) {
    /**
     * After this function there's a close brace
     */
    function bar() {
    }
}

Your way outputs:

function bar() {
}
}

My way outputs:

    function bar() {
    }
}

Neither is "Right" but I think perhaps the later is more expected? A conscious decision needs to be made

At some point I'd love to re-write to esprima. The main problem with that though is that you then can't see docs for code that doesn't compile.

Owner

tj commented Jul 3, 2012

yeah doing it with esprima would be nice, it's definitely iffy when it comes to nested functions

Contributor

ForbesLindesay commented Jul 3, 2012

Hmm, maybe at some point if I get time. In the mean time, would it be possible to either accept these changes, or make the changes you've suggested (then close this pull request). I'm not too bothered either way tbh, then hopefully at some point I'll find time to build the esprima version.

Contributor

ForbesLindesay commented Sep 15, 2012

When we build the esprima version (or even now) we should really use http://constellation.github.com/doctrine/demo/index.html to parse the comments, it seems much more comprehensive a match to the google closure spec (https://developers.google.com/closure/compiler/docs/js-for-compiler) than what we currently do.

+1 on re-doing in esprima :) Especially if we can support the esprima-moz branch (cough) also.

Owner

tj commented Jan 7, 2013

largely i dont think it would be necessary to rewrite because documenting enclosed functions like that is just kinda weird IMO, we shouldn't really be writing public js like that anyway

Contributor

ForbesLindesay commented Jan 7, 2013

That's true for documenting public JS, but you could also use this to document your internal code and help new/inexperienced developers get up to speed quickly.

In any case, if we used something like esprima, it might be possible to produce documentation that's based on the structure that actually gets assigned to module.exports in a much more reliable way than the current regexp parser.

Collaborator

evindor commented Sep 4, 2014

Merged in #128.

@evindor evindor closed this Sep 4, 2014

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