Failing Test For Inheritance Indentation inside a replacement variable.

[mikesherov]

Thanks again for all your hard work!

[bobthecow]

There's a quick fix for this, but making that change didn't make any tests fail so I'm not sure we have test coverage for why it was indenting in the first place.

Should a block (or block replacement) ever be indented? Does this mess up blocks inside partials or something?

[mikesherov]

{{$id}}content{{/id}} should always be replaced as is. I appreciate the due diligence, just looks like I added a missing test that illustrates the problem is all. It likely indents because of a bug in the "add whitespace when a partial is the only thing on a line" bug that we addressed in 2.7.0

[mikesherov]

Also, it's hard to tell from the commit log, but looks like only changes since 2.7.0 are cleaning up whitespace? If so, and this gets fixed soon, could you cut a patch release for this? And if not, is it generally safe for me to use the exact SHA of the fix until you cut a patch release?

[bobthecow]

There's definitely whitespace weirdness in template inheritance that we're not accounting for. See b3d80b9, for example.

That said, fixing this issue seems to make it closer to correct, so I'm okay pushing a fix for this … but possibly not cutting a stable release until it's fully addressed?

[bobthecow]

Okay. There's a fix (for just your issue) in 7cb9fdf. Feel free to use that exact SHA for now, as I don't expect to get a stable release out until this is resolved a bit more fully.

[bobthecow]

@jazzdan, @dtb, @mdg, @benburry and anyone else at @etsy… thoughts on this?

[jazzdan]

Hey @bobthecow we've started to encounter some of these whitespace issues as well. We haven't upgraded to 2.7.0 yet, we'll keep you apprised when we do. :)

[bobthecow]

@jazzdan Thanks. Note that this is a potential fix (which would go in 2.8.0) not one that's released yet :)

[jazzdan]

@bobthecow right, but we haven't even gotten the whitespace fix that went into 2.7.0, so I'm not sure which we're experiencing. I haven't done much investigation yet.

[bobthecow]

Gotcha.

[bobthecow]

Wait. 2.7.0 was the template inheritance release. You're not even using the stable release version of that yet?

[bobthecow]

@mikesherov You're correct, BTW. The only things in dev that haven't been released are code style changes and a bit of internal cleanup. Nothing that would affect you in the least.

[jazzdan]

@bobthecow heh... yeah we're actually on my branch still. I'll be working on it soon!

[mikesherov]

@bobthecow, why would the fix warrant a 2.8.0? Wouldn't this fix be a patch to 2.7.1?

[bobthecow]

@mikesherov It is a bug fix, so a patch would be appropriate. I feel that, since it's a fix for a bug that's been in the library for over six months, it would also be appropriate to release it as a point release, just to signify that something bigger happened, and you might want to check a little more thoroughly in case you've been relying on the bug in production :)

Given that SemVer doesn't place limits on version bumps (i.e. you can bump whenever you want, even if you're not required to), and how long the bug has been in the wild, I'd prefer to release it as a point release. Make sense?

[mikesherov]

Sure. Whatever version you want to release is your choice. I was just saying 2.7.1 because I selfishly want this patch released and I personally like releasing early and often.

To me, when it's a bug fix with no new features, I folllow semver strictly and mark it as a patch. If this is truly a non-BC change, I'd mark as major.

Anyway, whatever you choose is fine by me, as I've already moved onto using the exact SHA you provided. Keep up the great work!

[bobthecow]

It's not really a BC break, because it's a bugfix. That's allowed for in SemVer, and makes sense. The only reason I'd hesitate making it a point release is that the bug has existed for quite a while, and people are likely either relying on it or working around it. Making this v2.8.0 wouldn't mean it gets released any sooner or later, though :)