Skip to content

Conversation

@jeffshaver
Copy link
Contributor

If there is any more info you need, please let me know.

I assume that i will need to add some tests maybe, but I didn't see the correct place to put them. Guidance there?

@orktes
Copy link
Owner

orktes commented Jan 4, 2016

Hi!

Thanks for contributing!

Indentation test file is located in
https://github.com/orktes/atom-react/blob/master/spec/fixtures/sample.jsx
and the expected result in
https://github.com/orktes/atom-react/blob/master/spec/fixtures/sample-formatted.jsx

So adding something like

// sample.jsx
<div>
<span 
foo="bar"
bar="foo"
/>
<span barfoo="123" />
<span>
foo
</span>
</div>

// sample-formatted.jsx
<div>
  <span 
    foo="bar"
    bar="foo"
  />
  <span barfoo="123" />
  <span 
    foo="bar"
    bar="foo" />
  <span>
    foo
  </span>
</div>

ps. currently it seems that one tokenizer test is failing against the latest lang-javascript package that atom-react is extending with JSX. Feel free to ignore that for now.

@orktes
Copy link
Owner

orktes commented Jan 4, 2016

ps ignore appveyor (I should remove that as it causes more trouble than it helps)

@jeffshaver
Copy link
Contributor Author

@orktes thanks for the quick response!

Will work this later today (hopefully). Just to be clear in regards to #131, this isn't really reading the eslint config. I guess it may be possible to do that, but I didn't think it was necessary. This seems more like a bug than preference (to me, but I don't know if everyone writes jsx like me).

This is basically what I end up with when I use self-closing tags right now:

<MyComponent
  id="a"
  {...this.props}
  /> // Note how this is indented

All this pr does is this:

<MyComponent
  id="a"
  {...this.props}
/> // This line gets auto-unindented now

@orktes
Copy link
Owner

orktes commented Jan 4, 2016

Yeap. My own preference has been to close after last attribute but
reasoning for putting it onto a separate line is clear.

I added a separate issue about the eslint conf stuff.

On Monday, 4 January 2016, Jeffrey E. Shaver II notifications@github.com
wrote:

@orktes https://github.com/orktes thanks for the quick response!

Will work this later today (hopefully). Just to be clear in regards to
#131 #131, this isn't really
reading the eslint config. I guess it may be possible to do that, but I
didn't think it was necessary. This seems more like a bug than preference
(to me, but I don't know if everyone writes jsx like me).

This is basically what I end up with when I use self-closing tags right
now:

<MyComponent
id="a"
{...this.props}
/> // Note how this is indented

All this pr does is this:

<MyComponent
id="a"
{...this.props}
/> // This line gets auto-unindented now


Reply to this email directly or view it on GitHub
#131 (comment).

Ystävällisin terveisin,
Jaakko Lukkari

@jeffshaver
Copy link
Contributor Author

@orktes I changed sample.jsx and sample-formatted.jsx and ran apm test. Only the one failed as you mentioned above.

@jeffshaver
Copy link
Contributor Author

@orktes is there something else I need to do here?

@jeffshaver
Copy link
Contributor Author

@orktes anything else on this?

@orktes
Copy link
Owner

orktes commented Jan 13, 2016

Hi sorry for the delay. Been terribly busy with work. All good!

orktes added a commit that referenced this pull request Jan 13, 2016
Fixes #36. Add condition to decreaseIndentPattern regex to support self closing-tags
@orktes orktes merged commit 456c704 into orktes:master Jan 13, 2016
@orktes
Copy link
Owner

orktes commented Jan 15, 2016

caused regression had to revert so we can release more urgent 1.4 related fixes.

orktes pushed a commit that referenced this pull request Jan 15, 2016
This reverts commit 456c704, reversing
changes made to b91f048.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants