TylerBrock / mongo-hacker

MongoDB Shell Enhancements for Hackers
tylerbrock.github.io/mongo-hacker
MIT License
1.79k stars 235 forks source link

DRY up formatting padded/aligned columns #134

Closed pvdb closed 9 years ago

pvdb commented 9 years ago

Hey @TylerBrock,

This PR doesn't introduce any functional changes, but rather DRYs up similar, almost identical code that existed in the implementation of the following commands:

By introducing the printPaddedColumns() helper function, I was able to significantly simplify the implementation of these 3 commands, as the helper provides the formatting of padded/aligned columns, so that the commands only need to implement their respective logic without having to concern themselves about formatting their output.

The printPaddedColumns(keys, values) helper works as follows:

The refactoring isn't completely backwards compatible: the layout of these 3 commands changes slightly, as illustrated by the below before (left) & after (right) screenshot, but arguably the layout is now more consistent and slightly easier to peruse.

before_after

Thanks,

@pvdb

pvdb commented 9 years ago

BTW, @TylerBrock, the main driver for this refactor is that implementing the new count collections helper to which I alluded in #133 is now a doddle (so expect another PR shortly after this one gets signed off :smile:):

    if (what == "collections" || what == "tables") {
        databaseNames = db.getMongo().getDatabaseNames();
        collectionCounts = databaseNames.map(function (databaseName) {
            var count = db.getMongo().getDB(databaseName).getCollectionNames().length;
            return (count.commify() + " collection(s)");
        });
        printPaddedColumns(databaseNames, collectionCounts);
        return "";
    }
TylerBrock commented 9 years ago

Cool, thank you! The output of show collections needs some work though. Is there any way we can space things out so the slashes align vertically?

I'm not sure I'm a fan of the arrow. Is that part of standard terminal font sets? I'm running a powerline font so I can't tell anymore what is usual or not :open_mouth:

The benefit for show dbs is marginal. The real advantage is for something like the example you have for count documents so I think this has promise.

pvdb commented 9 years ago

Is there any way we can space things out so the slashes align vertically?

Have a look at 347c2b4 ... that results in:

macpvandenb(mongod-3.0.3) huge> show collections
greasy_spoons  →  645.135MB /  817.719MB
restaurants    → 6872.405MB / 8874.832MB
system.indexes →    0.000MB /    0.008MB
macpvandenb(mongod-3.0.3) huge> _
pvdb commented 9 years ago

I'm not sure I'm a fan of the arrow. Is that part of standard terminal font sets?

The arrow is a UTF-8 character, so won't work in ASCII-only terminals, but then BSON is pretty much UTF-8 so I figured that'd be OK... having said that, though, it's easy enough to change, so maybe I can change it to "-" or some other ASCII separator?

TylerBrock commented 9 years ago

Greatness. Lets merge it in. Thanks for getting back to me so quickly.

pvdb commented 9 years ago

so I think this has promise.

:thumbsup: :smile: