Skip to content

Conversation

@busunkim96
Copy link
Contributor

@busunkim96 busunkim96 commented Apr 12, 2021

Enable trim_blocks and lstrip_blocks in the Jinja environment to make it easier to manage whitespace and newlines. This PR may result in slight changes to the formatting of generated code.

https://jinja.palletsprojects.com/en/2.11.x/api/#jinja2.Environment
https://jinja.palletsprojects.com/en/2.11.x/templates/#whitespace-control

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 12, 2021
@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #838 (9d0d188) into master (7c185e8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #838   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        27    +1     
  Lines         1608      1653   +45     
  Branches       328       337    +9     
=========================================
+ Hits          1608      1653   +45     
Impacted Files Coverage Δ
gapic/schema/metadata.py 100.00% <ø> (ø)
gapic/utils/case.py 100.00% <ø> (ø)
gapic/utils/reserved_names.py 100.00% <ø> (ø)
gapic/generator/generator.py 100.00% <100.00%> (ø)
gapic/schema/api.py 100.00% <100.00%> (ø)
gapic/schema/wrappers.py 100.00% <100.00%> (ø)
gapic/utils/__init__.py 100.00% <100.00%> (ø)
gapic/utils/checks.py 100.00% <100.00%> (ø)
gapic/utils/options.py 100.00% <100.00%> (ø)

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 89d6f35...9d0d188. Read the comment docs.

@software-dov
Copy link
Contributor

How's the result?

@busunkim96
Copy link
Contributor Author

@software-dov Still working out issues in a few places (hence the failing tests :)), but I think this should result in fewer instances of manual whitespace trimming {%-} and {-%}. https://jinja.palletsprojects.com/en/2.11.x/templates/#whitespace-control was the inspiration.

@busunkim96 busunkim96 changed the title fix: use trim_blocks and lstrip_blocks for jinja templates chore: use trim_blocks and lstrip_blocks for jinja templates Apr 13, 2021
@busunkim96 busunkim96 requested a review from software-dov April 13, 2021 17:11
@busunkim96 busunkim96 marked this pull request as ready for review April 13, 2021 17:11
@busunkim96 busunkim96 requested a review from a team as a code owner April 13, 2021 17:11
@software-dov
Copy link
Contributor

More power to you for doing this. I remember having screaming fits when I first tried to make vaguely-python-whitespace templates.

@busunkim96 busunkim96 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 13, 2021
@busunkim96
Copy link
Contributor Author

Will get #774 over the line first and then merge this, since I suspect there will be merge conflicts otherwise.

@busunkim96 busunkim96 force-pushed the lstrip-and-trim-blocks branch from 264ca2d to 9d0d188 Compare April 22, 2021 15:39
@busunkim96 busunkim96 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 22, 2021
@busunkim96 busunkim96 merged commit e620d71 into master Apr 22, 2021
@busunkim96 busunkim96 deleted the lstrip-and-trim-blocks branch April 22, 2021 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants