[Review] Jinja2-like whitespace trim behavior

[bejar37]

This has been attempted at least twice in #85 and #32 .

In #89 you expressed interest in the way this is done in Jinja2, I've attempted to recreate that here. This is still Review because I'm waiting on feedback before adding:

• options to Environment to set the default trim behaviors
• documentation, more test cases, etc.

[bejar37]

Oops, I see NSRegularExpression is unavailable on Linux... could do this with the Posix regex implementation like in SwiftyRegex or use other available string methods.

[ikesyo]

corelibs-foundation has the implementation but mistakenly named as RegularExpression on Swift 3.0.x. That should be renamed as NSRegularExpression on Swift 3.1. In the meantime you can have the following workaround:

#if os(Linux)
typealias NSRegularExpression = RegularExpression
#endif

[kylef]

corelibs-foundation has the implementation but mistakenly named as RegularExpression on Swift 3.0.x. That should be renamed as NSRegularExpression on Swift 3.1. In the meantime you can have the following workaround:

If it's different per version, perhaps that conditional could be per Swift version so this will continue to work for both Swift versions?

[bejar37]

@ikesyo cool, thanks! @kylef 3fc963b introduces a typealias for NSRegularExpression in Linux when Swift < 3.1.

[bejar37]

I realized that I could add support for all block tokens automatically by keeping the whitespace state inside of the Parser, rather than passing it in when calling parse. Pushed an update 0a93d76.

[yonaskolb]

@bejar37 from the discussion in #32 and #85 we think your solution is the best. Whether this should have an environment property to set the default behaviour is still up for debate, but for what it's worth I would love that feature.

[yonaskolb]

Are there any updates on this?

[bejar37]

@yonaskolb @AliSoftware @kylef I'd love to get this pulled and am willing to do the work to do so this weekend.
I'll add an environment variable to control the trimming behavior and rebase this on master.

A review would be appreciated!

I'll look at Jinja or other templating solutions for a good name for the environment variable.

[bejar37]

Update - I did the required work to get this building with the latest master.

I decided to hold off on adding the environment variable since there isn't consensus that it's wanted, and it makes sense to me to add it in a follow on PR when and if this one is merged.

[yonaskolb]

Would love to see this get merged

[DanielAsher]

I've got some seriously funky stencil formatting in order to get output whitespace to be correct. No fun reading the many >400 character lines stencil authors are required to write.

[yonaskolb]

@DanielAsher agreed. @kylef or @ilyapuchka do you have any comments on @bejar37's approach here? Has anything relevant changed since this was last rebased?

[ilyapuchka]

@yonaskolb I think its fine, though myself I don't see the reason to have {+ tag, keeping spaces as they are should be default behaviour without needing any different tag. In swift templates in Sourcery there are two different tags to remove white spaces (trailing or leading) and another to remove trailing new lines.

[ilyapuchka]

@kylef @yonaskolb @AliSoftware @djbe can you guys look at this any time soon?

[DanielAsher]

The lack of movement on this PR is so frustrating for metaprogramming with stencil. Ordinary indenting and source formatting of stencil files is completely impossible without totally munging the output. I don't want to rewrite my stencils in .swifttemplate yet, but the painstaking process of formatting and unformatting make me reconsider. Every time.

[djbe]

@ilyapuchka As you've probably noticed, I'm working through reviewing some of the PRs, mostly in an order of first those "I get why" and that I understand (as I'm not so familiar with the codebase as with SwiftGen's).

Just to say that I'll get to this one soon-ish.

[ilyapuchka]

@kylef @djbe @yonaskolb will it make sense if I try to update this? Or we should just close all related issues and forget about this like it never happened? 🙄

[yonaskolb]

@ilyapuchka yeah I’d love for this to get merged. It’s the last holdout for me to use this over my fork. For my purposes I’d need to have this on by default via a setting on environment

[johntmcintosh]

I was hoping to improve readability in some templates that are getting increasingly complex and came across this PR that appears to be the desired approach for managing line break formatting. @kylef @djbe is this still on your radar for reviewing?

[acecilia]

@djbe @kylef your feedback here will be appreciated. If you do not have time for pushing this forward, it will help if you drop two words mentioning it. Thanks :)

[hlung]

I'm waiting for this!!!

Meanwhile on my machine...

extension String {
  func removingExcessiveNewLinesInStencilOutput() -> String {
    return self
      .trimmingCharacters(in: .whitespacesAndNewlines)
      .replacingOccurrences(of: "\n  \n", with: "\n")
      .replacingOccurrences(of: "\n    \n", with: "\n")
      .replacingOccurrences(of: "\n      \n", with: "\n")
      .replacingOccurrences(of: "\n        \n", with: "\n")
      .replacingOccurrences(of: "\n          \n", with: "\n")
  }
}

[yonaskolb]

I was working on this last night. Will get something up hopefully tonight. It includes the behaviour in this PR as well as a way to set a default behaviour in the environment

[acecilia]

To go around this issue I switched to mustache and followed the workaround explained in groue/GRMustache#46 (comment). Same workaround can be followed in stencil.

[yonaskolb]

Closing in favour of #287 which builds upon this. Thanks for the great work @bejar37!