style: Use dedent for prettier multi-line template literals #242

sudo-suhas · 2017-08-30T06:51:09Z

This PR uses dedent for beautifying multi-line template literals:

const dedent = require('dedent');
// ...

// Before
console.log(`This is a multi-line
sentence`);

// After
console.log(dedent`
  This is a multi-line
  sentence
`)

codecov · 2017-08-30T06:54:30Z

Codecov Report

Merging #242 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
+ Coverage   97.09%   97.15%   +0.06%     
==========================================
  Files          10       10              
  Lines         172      176       +4     
  Branches       26       26              
==========================================
+ Hits          167      171       +4     
  Misses          5        5

Impacted Files	Coverage Δ
src/getConfig.js	`97.14% <ø> (ø)`	⬆️
src/index.js	`88.88% <100%> (+1.01%)`	⬆️
src/runScript.js	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3fa4f6d...af45ede. Read the comment docs.

okonet

👍

okonet · 2017-08-30T07:42:24Z

Do you mind waiting till I merge #141 and applying those changes again? I just spent an evening merging master into it and I think this PR will intro some conflicts again. 🙄

sudo-suhas · 2017-08-30T07:44:11Z

Sure, no problem. ~~Please ping here once that is done.~~ I subscribed to that PR.

sudo-suhas · 2017-08-30T15:55:18Z

Invitation to join okonet/lint-staged from okonet

@okonet Thanks!

okonet · 2017-08-30T15:59:35Z

@sudo-suhas it's a pleasure! 👍 Thanks for accepting. I don't mind more eyes and hands here :)

okonet · 2017-09-04T12:55:37Z

@sudo-suhas do you need help with resolving conflicts in this one?

sudo-suhas · 2017-09-04T12:56:23Z

Nah.. I'll just discard my changes and start over.

luftywiranda13 · 2017-09-04T12:57:57Z

@sudo-suhas do you use cmd or powershell on windows? I'm thinking about something to improve our cross-platform compatibility

sudo-suhas · 2017-09-04T12:59:43Z

I actually use cmder. Helps me forget that I am on windows just a little bit 😆. But I can try out changes on cmd or powershell if needed.

sudo-suhas · 2017-09-07T04:50:11Z

I know this is less important but I figured since the PR was open and changes were simple, would be better to merge it in.

sudo-suhas · 2017-09-07T07:26:58Z

Rebasing and fixing conflicts.

okonet · 2017-09-07T07:44:03Z

test/__snapshots__/getConfig.spec.js.snap

-  }
-
-  will fix it and remove this message.
+Unknown option \\"*.js\\" with value ['eslint --fix', 'git add'] was found in the config root.


Is there a way to keep this indentation of 2 spaces?

It is possible if we keep a header text:

const message = dedent` WARNING Unknown option ${chalk.bold(`"${option}"`)} with value ${chalk.bold( format(config[option], { inlineCharacterLimit: Number.POSITIVE_INFINITY }) )} was found in the config root. You are probably trying to mix simple and advanced config formats. Adding ${chalk.bold(`"linters": { "${option}": ${JSON.stringify(config[option])} }`)} will fix it and remove this message. `

Dedent uses the min of indentation to strip. So we need relative indentation. What do you suggest?

Can we put 2 spaces on the first line:

const message = dedent` Unknown option ${chalk.bold(`"${option}"`)} with value ${chalk.bold( format(config[option], { inlineCharacterLimit: Number.POSITIVE_INFINITY }) )} was found in the config root. You are probably trying to mix simple and advanced config formats. Adding ${chalk.bold(`"linters": { "${option}": ${JSON.stringify(config[option])} }`)} will fix it and remove this message. `

Although it doesn't look very nice either...

How about

const message = dedent` UNKNOWN OPTION Option ${chalk.bold(`"${option}"`)} with value ${chalk.bold( format(config[option], { inlineCharacterLimit: Number.POSITIVE_INFINITY }) )} was found in the config root. You are probably trying to mix simple and advanced config formats. Adding ${chalk.bold(`"linters": { "${option}": ${JSON.stringify(config[option])} }`)} will fix it and remove this message. `

All caps are bad for readability. Also, I don't see a reason to have 2 titles. If dedent doesn't support our case, we should either not use it or add support to dedent. I don't want to compromise the UX in favor of better indented codebase.

Okay. That makes sense. I'll look into dedent.

@okonet Small update, I made a PR to dedent which would allow us to specify default indent - dmnd/dedent#14.

okonet · 2017-09-07T07:44:26Z

test/__snapshots__/index.spec.js.snap

-Running lint-staged with the following config:
-{
+LOG Running lint-staged with the following config:
+         {


That's an odd indentation here

You're right, sorry I missed it. Can be fixed with this:

console.log(`Running lint-staged with the following config: \n${stringifyObject(config)}`)

Is this ok?

Okay with me!

luftywiranda13 · 2017-09-07T10:50:25Z

src/index.js

-Make sure you have created it. See https://github.com/okonet/lint-staged#readme.
+      console.error(dedent`
+        Could not parse lint-staged config.
+        Make sure you have created it. See https://github.com/okonet/lint-staged#readme.


isn't it better if we set the link to https://github.com/okonet/lint-staged#configuration?

or just https://github.com/okonet/lint-staged

Configuration is better IMO. No need to search for it.

okonet · 2017-10-11T04:44:20Z

Can this one be finalized, please?

sudo-suhas · 2017-10-11T04:46:08Z

I made a PR to dedent(dmnd/dedent#14) for our requirement but did not get a response yet.

sudo-suhas · 2017-10-29T06:04:58Z

@okonet I am not getting any response for the PR I made to dedent. We have a couple of options:

Don't use dedent for the section where we want some indentation.
Publish the package under a different name and allow for specifying indent.

What do you suggest?

okonet · 2017-11-06T10:22:40Z

@sudo-suhas can we just skip it for https://github.com/okonet/lint-staged/pull/242/files#diff-3aafb296b115847cd397630c3d4960a7R65 and move along?

sudo-suhas · 2017-11-06T10:24:58Z

Yes @okonet that's what I meant by

Don't use dedent for the section where we want some indentation.

Will rebase and resolve conflicts.

okonet · 2017-11-08T06:26:26Z

🍾

okonet previously approved these changes Aug 30, 2017

View reviewed changes

luftywiranda13 mentioned this pull request Sep 4, 2017

fix: Fix unicode symbols compatibility on Windows #248

Merged

sudo-suhas dismissed okonet’s stale review via 6834691 September 7, 2017 04:35

sudo-suhas force-pushed the style/dedent-template-literals branch from 28121aa to 6834691 Compare September 7, 2017 04:35

sudo-suhas force-pushed the style/dedent-template-literals branch from 6834691 to 22bb370 Compare September 7, 2017 07:30

sudo-suhas requested a review from okonet September 7, 2017 07:40

okonet requested changes Sep 7, 2017

View reviewed changes

luftywiranda13 reviewed Sep 7, 2017

View reviewed changes

style: Use dedent for prettier multi-line template literals

af45ede

sudo-suhas force-pushed the style/dedent-template-literals branch from 22bb370 to af45ede Compare November 8, 2017 03:40

sudo-suhas requested a review from okonet November 8, 2017 03:47

okonet approved these changes Nov 8, 2017

View reviewed changes

okonet merged commit 0e40d2c into lint-staged:master Nov 8, 2017

sudo-suhas deleted the style/dedent-template-literals branch November 8, 2017 06:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

style: Use dedent for prettier multi-line template literals #242

style: Use dedent for prettier multi-line template literals #242

sudo-suhas commented Aug 30, 2017

codecov bot commented Aug 30, 2017 •

edited

okonet left a comment

okonet commented Aug 30, 2017

sudo-suhas commented Aug 30, 2017 •

edited

sudo-suhas commented Aug 30, 2017

okonet commented Aug 30, 2017

okonet commented Sep 4, 2017 •

edited

sudo-suhas commented Sep 4, 2017

luftywiranda13 commented Sep 4, 2017

sudo-suhas commented Sep 4, 2017 •

edited

sudo-suhas commented Sep 7, 2017

sudo-suhas commented Sep 7, 2017

okonet Sep 7, 2017

sudo-suhas Sep 7, 2017

okonet Sep 7, 2017

okonet Sep 7, 2017

sudo-suhas Sep 7, 2017

okonet Sep 7, 2017

sudo-suhas Sep 7, 2017

sudo-suhas Sep 24, 2017

okonet Sep 7, 2017

sudo-suhas Sep 7, 2017

okonet Sep 7, 2017

luftywiranda13 Sep 7, 2017

luftywiranda13 Sep 7, 2017

okonet Sep 7, 2017

okonet commented Oct 11, 2017

sudo-suhas commented Oct 11, 2017

sudo-suhas commented Oct 29, 2017

okonet commented Nov 6, 2017

sudo-suhas commented Nov 6, 2017

okonet commented Nov 8, 2017

style: Use dedent for prettier multi-line template literals #242

style: Use dedent for prettier multi-line template literals #242

Conversation

sudo-suhas commented Aug 30, 2017

codecov bot commented Aug 30, 2017 • edited

Codecov Report

okonet left a comment

Choose a reason for hiding this comment

okonet commented Aug 30, 2017

sudo-suhas commented Aug 30, 2017 • edited

sudo-suhas commented Aug 30, 2017

okonet commented Aug 30, 2017

okonet commented Sep 4, 2017 • edited

sudo-suhas commented Sep 4, 2017

luftywiranda13 commented Sep 4, 2017

sudo-suhas commented Sep 4, 2017 • edited

sudo-suhas commented Sep 7, 2017

sudo-suhas commented Sep 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okonet commented Oct 11, 2017

sudo-suhas commented Oct 11, 2017

sudo-suhas commented Oct 29, 2017

okonet commented Nov 6, 2017

sudo-suhas commented Nov 6, 2017

okonet commented Nov 8, 2017

codecov bot commented Aug 30, 2017 •

edited

sudo-suhas commented Aug 30, 2017 •

edited

okonet commented Sep 4, 2017 •

edited

sudo-suhas commented Sep 4, 2017 •

edited