Skip to content
This repository has been archived by the owner on Apr 26, 2021. It is now read-only.

Remove left & right trimming? #71

Closed
matthewmueller opened this issue Aug 15, 2016 · 10 comments
Closed

Remove left & right trimming? #71

matthewmueller opened this issue Aug 15, 2016 · 10 comments

Comments

@matthewmueller
Copy link

Sorry one more thing that would be nice to have. Right now the library trims the left and right spacing from the input. The thing I love about this library is that you can sort of make your CLI however you want with just a few standard expressions. For my CLI, I'm finding that adding a bit of spacing would make things a bit easier to read.

@felixSchl
Copy link
Owner

Agreed. Maybe a python like textwrap.dedent would be fair? Would you be able to provide a simple example? And lastly, why would you care about trailing space on the right?

@matthewmueller
Copy link
Author

matthewmueller commented Aug 16, 2016

i think you could use https://github.com/sindresorhus/strip-indent to do a dedent. here's what I'm working with:

screen shot 2016-08-16 at 10 44 14 am

Much better than the output of commander, but I'd love to give it a bit of padding :-)

@felixSchl
Copy link
Owner

That's one good looking CLI. Makes me wonder what does your neodoc source string look like to get all the colors going? I'd imagine a lot of these guys: ${chalk.blue("usage:")}. Makes me think if we need to add colouring support into neodoc to highlight section titles and comments. But that's a separate issue. In terms of this, to add the padding, why don't we just strip all leading and trailing empty lines but keep the left indent as-is. I think that would be suitable.

@felixSchl
Copy link
Owner

Do you mind providing a simple example for this? I have checked the sources, I am actually not de-denting the --help output, only the shortened usage section help text. The only thing I do is trimming the string:

ShowHelp help -> abort 0 (String.trim help)
.

@felixSchl felixSchl modified the milestone: Next release Aug 17, 2016
@matthewmueller
Copy link
Author

matthewmueller commented Aug 17, 2016

Certainly, here's a trimmed version of the code:

thimble

#!/usr/bin/env node

const { run } = require('neodoc')
const chalk = require('chalk')
const argv = run(`

  ${chalk.inverse(' THIMBLE ')} A scaffolding system that grows with you.

  ${chalk.blue('I. Usage')}:

    thimble <command> [<args>...]
    thimble -h | --help
    thimble --version


`)

run:

./thimble --help

You'll notice that despite having some extra padding and whitespace on both sides, it'll still hug the left side. It's totally your call on what you think is best, but I think since most people will have this code top-level (as opposed to inside an if statement block), it won't be unexpected to not dedent.

@matthewmueller
Copy link
Author

matthewmueller commented Aug 17, 2016

On second thought, I definitely think you should just leave the dedenting, trimming up to the implementer of the CLI.

  • If I don't want left padding: run(myCLI.trimLeft())
  • If I don't want right padding: run(myCLI.trimRight())
  • If I don't want any padding: run(myCLI.trim())
  • If I want dedent, I can use strip-indent and do run(strip(myCLI))

This probably allows you to reduce the codebase a bit.

@felixSchl
Copy link
Owner

felixSchl commented Aug 17, 2016

Thank you for providing the example and I agree with your reasoning. But what do you think about removing the very first (and maybe last) newline character? This way if you want padding, you just add another line. E.g.:

run(`
usage: foo bar
`)
run(`

usage: foo bar

`)

I've updated the JS test bed to able to test these things and have made corresponding adjustments to the code, but it would be good if we could agree on this. In part I disagree with myself with the above because the behavior is implicit and cannot be turned off. However, I think it would make it more usable and avoids extra function call clutter or even bringing in another dependency.

Edit: To re-iterate: what I mean is that this approach would be less radical than trimming to the first non-ws character (only remove first and last empty line) and it would not mess with the indentation at all.

felixSchl added a commit that referenced this issue Aug 18, 2016
This also adds chalk to the project as a devDependency.
felixSchl added a commit that referenced this issue Aug 18, 2016
felixSchl added a commit that referenced this issue Aug 18, 2016
@felixSchl
Copy link
Owner

I've implemented the above behavior and adjusted the test case:

neodoc/test-js/test.js

Lines 195 to 213 in a1b375f

describe('#71 - remove left & right trimming', () => {
it('should not trim the help text', () => {
const { stdout } = runFakeProc(() => {
neodoc.run(`
${chalk.inverse(' THIMBLE ')} A scaffolding system that grows with you.
${chalk.blue('I. Usage')}:
thimble <command> [<args>...]
thimble -h | --help
thimble --version
`, { argv: [ '--help' ] });
});
expect(stdout).to.equal(
` ${chalk.inverse(' THIMBLE ')} A scaffolding system that grows with you.
${chalk.blue('I. Usage')}:
thimble <command> [<args>...]
thimble -h | --help
thimble --version`);
});
});

It just boils down to this regular expression:

trimHelp = Regex.replace (regex' "(^\\s*(\r\n|\n|\r))|((\r\n|\n|\r)\\s*$)" "g") ""

...which reminds me of http://stackstatus.net/post/147710624694/outage-postmortem-july-20-2016 :/ But I think our inputs are small enough and in complete control of the user that this is non-issue.

felixSchl added a commit that referenced this issue Aug 18, 2016
This also adds chalk to the project as a devDependency.
@matthewmueller
Copy link
Author

matthewmueller commented Aug 18, 2016

But what do you think about removing the very first (and maybe last) newline character?

Totally agree! Forgot about the first and last \n

Awesome, thanks for your hard work!

@felixSchl
Copy link
Owner

Sweet, I'll merge this then and create a patch release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants