zspecza / common-tags

🔖 Useful template literal tags for dealing with strings in ES2015+
Other
1.99k stars 61 forks source link

Fix InlineArrayTransformer adding extra newlines when maintaining indentation [WIP] #114

Closed ibrahima closed 6 years ago

ibrahima commented 7 years ago

The latter test tests behavior that I think should be correct, but it fails because it is not the current behavior.

codecov-io commented 7 years ago

Codecov Report

Merging #114 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #114   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          23     23           
  Lines         149    149           
=====================================
  Hits          149    149
Impacted Files Coverage Δ
...c/inlineArrayTransformer/inlineArrayTransformer.js 100% <100%> (ø) :arrow_up:

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 9295ee1...cb95d00. Read the comment docs.

fatfisz commented 6 years ago

Hi, what a good catch!

Answering your question from #113: I think using a negation in this case is just fine because it should be future-proof.

fatfisz commented 6 years ago

@ibrahima I don't have permissions to re-run the build, but OTOH you could add a commit that adds you to contributors in package.json. Could you please do that and then we could go ahead with the merge if everything's green?

ibrahima commented 6 years ago

I just pushed a merge commit; I feel weird adding myself to contributors in package.json since it's a small patch, and not all the existing contributors on Github are in there. But anyway, CI should run again now.

fatfisz commented 6 years ago

If that's how you feel, then ok. If you change your mind though, don't hesitate to get back to us - you actually found the root cause and spent some time on fixing the problem, which is much more than most people do. If this is not worthy a mention in package.json, then I don't know what is 😉

fatfisz commented 6 years ago

Released in v1.5.0!

ibrahima commented 6 years ago

Thanks!