Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trim whitespace #287

Merged
merged 3 commits into from
Jul 28, 2022
Merged

Trim whitespace #287

merged 3 commits into from
Jul 28, 2022

Conversation

yonaskolb
Copy link
Collaborator

This takes the work that @bejar37 started in #92 and builds upon it.

In addition to the Jinja2 trimming symbols, this adds a trimBehavior: TrimBehaviour on Environment. This defaults to .none but can be customized. It also comes with a built in .smart case that removes whitespace before a block, and whitespace and a single newline after a block.

@yonaskolb

This comment was marked as outdated.

Copy link
Collaborator

@ilyapuchka ilyapuchka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is amazing to see finally moving forward. I didn't go deep into implementation as I trust by this time it should be correct enough and tests look fine as well, only have few minor suggestions.
But we also need to update docs at least before this is released.

Sources/TrimBehavior.swift Outdated Show resolved Hide resolved
Sources/TrimBehavior.swift Outdated Show resolved Hide resolved
Sources/TrimBehavior.swift Outdated Show resolved Hide resolved
Sources/TrimBehavior.swift Outdated Show resolved Hide resolved
Sources/TrimBehavior.swift Outdated Show resolved Hide resolved
Sources/TrimBehavior.swift Outdated Show resolved Hide resolved
Tests/StencilTests/NodeSpec.swift Show resolved Hide resolved
Sources/Tokenizer.swift Outdated Show resolved Hide resolved
@aufflick

This comment was marked as outdated.

@tikimcfee

This comment was marked as off-topic.

@tomnvt

This comment was marked as outdated.

@subdan

This comment was marked as outdated.

@subdan

This comment was marked as outdated.

@thebarndog

This comment was marked as outdated.

@djbe djbe force-pushed the trim_whitespace branch from 8e327cf to 42fe9eb Compare July 27, 2022 04:43
@SwiftGen-Eve
Copy link

SwiftGen-Eve commented Jul 27, 2022

1 Message
📖 Big PR

Hey 👋 I'm Eve, the friendly bot watching over Stencil 🤖

Thanks a lot for your contribution!


Seems like everything is in order 👍 You did a good job here! 🤝

Generated by 🚫 Danger

@djbe djbe marked this pull request as draft July 27, 2022 04:44
@djbe
Copy link
Contributor

djbe commented Jul 27, 2022

I've rebased the changes on top of the (currently still open) drop swift4 PR, once that is merged, this should be converted to a full PR again done.

TODO:

  • add docs for the changes to tags (those -/+ characters within a tag)
  • add docs about the new global (environment) trim behaviours

@djbe djbe force-pushed the trim_whitespace branch from 42fe9eb to c3a4f96 Compare July 27, 2022 04:50
@djbe djbe added this to the 0.15.0 milestone Jul 27, 2022
@djbe djbe force-pushed the trim_whitespace branch 3 times, most recently from 7933fab to 07d9fb1 Compare July 28, 2022 01:16
@djbe djbe force-pushed the trim_whitespace branch from 07d9fb1 to 27a543d Compare July 28, 2022 14:43
@djbe djbe marked this pull request as ready for review July 28, 2022 14:44
Copy link
Contributor

@djbe djbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some rudimentary docs & linked to those from Jinja. PR should now be complete.

@djbe djbe enabled auto-merge July 28, 2022 16:25
@djbe djbe removed the request for review from AliSoftware July 28, 2022 16:25
@djbe djbe merged commit 4d3f911 into master Jul 28, 2022
@djbe djbe deleted the trim_whitespace branch July 28, 2022 16:25
@djbe djbe mentioned this pull request Jul 28, 2022
@yonaskolb
Copy link
Collaborator Author

Thank you @djbe for getting this across the line! Awesome work here in the repo in the last couple of days 💪

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.

9 participants