Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template string indentation style #1138

Closed
wll8 opened this issue May 21, 2018 · 5 comments

Comments

@wll8
Copy link

commented May 21, 2018

What version of standard?

  "devDependencies": {
    "babel-eslint": "^8.2.1",
    "eslint": "^4.15.0",
    "eslint-config-standard": "^10.2.1",
    "eslint-friendly-formatter": "^3.0.0",
    "eslint-loader": "^1.7.1",
    "eslint-plugin-import": "^2.7.0",
    "eslint-plugin-node": "^5.2.0",
    "eslint-plugin-promise": "^3.4.0",
    "eslint-plugin-standard": "^3.0.1",
    "eslint-plugin-vue": "^4.0.0",
  }

What operating system, Node.js, and npm version?
operating system Mac OS X 10.13.1
node v8.9.4
npm v5.6.0

What did you expect to happen?
I hope the style:

const fHtml = formatHtml(`
  <style>
    ${
        getText('./css1.css')
      + getText('./css2.css')
    }
  </style>
`)

What actually happened?
Get error:
[eslint] Expected indentation of 2 spaces but found 12. (indent)

Do I have to use the following style?

const fHtml = formatHtml(`
  <style>
    ${getText('./css1.css') + getText('./css2.css')}
  </style>
`)
const fHtml = formatHtml(`
  <style>
    ${
  getText('./css1.css')
+ getText('./css2.css')
}
  </style>
`)
@LinusU

This comment has been minimized.

Copy link
Member

commented May 21, 2018

I think that the problem is that you are basically writing JavaScript that looks like this:

const text = (
        getText('./css1.css')
      + getText('./css2.css')
    )

I don't have a strong opinion here, but I might be leaning towards not changing anything 🤔

btw. you could do it like this, no?

const fHtml = formatHtml(`
  <style>
    ${getText('./css1.css')}
    ${getText('./css2.css')}
  </style>
`)
@wll8

This comment has been minimized.

Copy link
Author

commented May 21, 2018

@LinusU

1

const text = (
    getText('./css1.css')
  + getText('./css2.css')
)

It can indeed be done. But I don't want to write more code and variables.
Just like jsx , why not use expressions? 😄

2

const fHtml = formatHtml(`
  <style>
    ${getText('./css1.css')}
    ${getText('./css2.css')}
  </style>
`)

😎 I think they are different, like...

${1} ${2} and ${1 + 2}

3

I think this is where we should discuss. 😔

image

@LinusU

This comment has been minimized.

Copy link
Member

commented May 21, 2018

The 1 wasn't meant as a suggestion, it was meant to show why your code is rejected by standard. My point was to illustrate that you probably wouldn't have thought it was strange that standard didn't like:

const text = (
        getText('./css1.css')
      + getText('./css2.css')
    )

The 2 was meant as a suggestion for your specific case, as it seemed like you were outputting text...


I could potentially see that this should be legal though:

const text = `
  <style>
    ${
      foo() +
      bar()
    }
  </style>
`

although to be honest, I'm not 100% sure that I think it looks that good 🤔

It's hard to think about without a valid use-case, I can't see a scenario where you would want to do arithmetic addition on a long list of variables inside a template string...

@wll8

This comment has been minimized.

Copy link
Author

commented May 22, 2018

@LinusU
Ok. I also don't want to use arithmetic addition. I just think it's an example to illustrate that ${a} ${b} and ${a + b} are different.

I am focused on explaining it:
${a} ${b} does not ignore indentation.

The 1 is indeed a scheme. Maybe I should use it.

Thank you。

@feross

This comment has been minimized.

Copy link
Member

commented May 23, 2018

The JSX indent rules have some bugs with edge cases like this, in my experience.

@wll8 I think for your use case, @LinusU's suggestion works perfectly fine. Even though, you're technically right that ${a} ${b} and ${a + b} are different, if ${a} ${b} works for your use case, then why not use it?

I'm going to try to change rules in standard v12 to make JSX indentation work better, if it's at all possible. In the meantime, I think we can close this issue.

@feross feross closed this May 23, 2018

@feross feross added the question label May 23, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Aug 21, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants
You can’t perform that action at this time.