Prevents indent on blank lines of partials.

[bdiz]

This pull request is to prevent indent in partials that have blank lines. It is needed when handlebars is used as a code generator and the spacing of the output is important or linted.

Here is the result of the test in the pull request before the fix:

The blank line in Yehuda's function has indent. This is not desirable. After the fix, this line is no longer indented.

• Please don't start pull requests for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
• Maintain the existing code style
• Are focused on a single change (i.e. avoid large refactoring or style adjustments in untouched code if not the primary goal of the pull request)
• Have good commit messages
• Have tests
• Have the typings (lib/handlebars.d.ts) updated on every API change. If you need help, updating those, please mention that in the PR description.
• Don't significantly decrease the current code coverage (see coverage/lcov-report/index.html)
• Currently, the 4.x-branch contains the latest version. Please target that branch in the PR.

[nknapp]

Sounds like a breaking change to me, and a pretty special use-case. It should at least be a feature switch.

Even then... Couldn't you just add a post-processing step like

const withoutEmptySpaces = string.replace(/^\s+$/gm,"")

or use prettier to format the result? Seems easier to me than changing Handlebars.

[bdiz]

Thanks for taking a look.

Here is some output which is bit more illustrative of our use case.

We really see this as a bug in the indentation algorithm. We feel it's not clean code to add indent on the blank line of the function above. Anybody generating code that will be linted would benefit from this fix.

I'm not understanding this being a breaking change. What use case would want indent for blank lines of a partial?

We could post-process, as you recommend, but we use this extensively at https://www.uvmgen.com and since we are reactively generating code on the front end, performance is important to us. This does not add any performance overhead to handlebars since it already has a condition checking for blank lines as it adds indent.

Adding indent originally (#858) needed the preventDefault feature switch because indentation was a new feature. If you consider this a bug fix as we do, a feature switch may not be needed.

[nknapp]

I have become very cautious with changes and very cautious with the assumption that "nobody would use Handlebars this way". The lack of a formal language specification makes it difficult to say what is a bug and what isn't. The large number of users makes it probable that one of them relies on exactly that behavior that you want to change, even if it seems insignificant to you and me.

You are right that when using Handlebars to create JavaScript, Java or alike, it seems natural to skip indent on blank lines. In Python, the majority also seems to prefer blank lines without indent. But there are some that say blank lines should be indented (https://stackoverflow.com/a/25002433).

Without any target file type in mind, treating blank lines and non-blank lines the same seems natural to me, so I don't consider this a bug.

I cannot say for sure that your change does or doesn't break any build, except by publishing a new version with the change. Then, if just one user complains, I will have to revert the change and publish a new version. The user may have had an hour of debugging until she finds out that a change in Handlebars is responsible for her test failures. And I have spent at least an hour publishing two patch versions (publishing is not completely automated yet).

But since this affects that language, I'd like to get @wycats opinion, too.

[nknapp]

Oh, it just came to my mind that if this is not a bug, than it is a new feature that changes the language. I promised @wycats not to allow any new features, until there is a formal language specification.

Another reason to delegate this decision to @wycats

[wycats]

@nknapp thanks for being cautious!

I tend to think that this is on the border between a feature and a bug, but you're right that it's very likely to end up being a breaking change.

I wouldn't be opposed to collecting use-cases for a new revision of the whitespace stripping algorithm (indentation sensitivity comes to mind as a justification for a revision), and in that context, I think this change would make sense.