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 trailing whitespace #38

Closed
jayvdb opened this issue Jun 6, 2018 · 9 comments
Closed

Trim trailing whitespace #38

jayvdb opened this issue Jun 6, 2018 · 9 comments
Assignees
Milestone

Comments

@jayvdb
Copy link
Member

jayvdb commented Jun 6, 2018

Similar to #36 , and the trailing blank line fixup, ..

When importing content from other providers, their trailing blank lines shouldnt be propagated into moban managed repositories.

e.g. github/gitignore#2728 and moremoban/pypi-mobans#22 (comment)

If trailing whitespace needs to be preserved, we can use jinja {% raw %} to keep it.

@chfw
Copy link
Member

chfw commented Jul 15, 2018

Hmmm good point. Can have a look.

@jayvdb
Copy link
Member Author

jayvdb commented Jan 24, 2019

This could be a simple template plugin, like copy.

When it needs to be added to a templated file, a pipeline could be used. i.e. the mobanfile can have an extra target for intermediate file, like how gmake works.

e.g. .jj2 -> .dirty.txt -> .txt

and the intermediate file could be removed by a delete rule (#167)

Or we might allow the same target to be listed twice, and then use two template engines consecutively (jinja, then whitespace-cleaner).

@jayvdb jayvdb self-assigned this Jan 24, 2019
@jayvdb jayvdb added this to the 0.4.0 milestone Jan 24, 2019
@chfw chfw added this to back-log in 0.7.x Jan 29, 2019
@jayvdb jayvdb moved this from back-log to post-release in 0.7.x Feb 4, 2019
@chfw
Copy link
Member

chfw commented Mar 9, 2019

We can introduce filter plugin but my worry is that current implementation does not respect(or guarantee) appearance order. So all is collected and aggregated to a dictionary: template type: its template targets. And then this dictionary is iterated through its corresponding template engine.

@jayvdb
Copy link
Member Author

jayvdb commented Mar 9, 2019

I think that problem needs to be solved, and solved using a 'gmake' like algorithm.

We should be able to find an existing 'solver' algorithm, maybe by looking in other tools like https://github.com/brushtechnology/fabricate , or https://github.com/mesonbuild/meson (but the solver might be in https://github.com/ninja-build/ninja )

SAT solvers are another approach, but not so well supported in Python and probably overkill . https://research.swtch.com/version-sat

@jayvdb
Copy link
Member Author

jayvdb commented Mar 9, 2019

We could have a very basic solution by adding a 'plugin order' config value to the yaml, so the user can specify

moban_plugin_order:
  - jinja
  - trim-whitespace
  - etc

@chfw
Copy link
Member

chfw commented Mar 9, 2019

I think ordereddict should be able to help us.

@chfw
Copy link
Member

chfw commented Mar 11, 2019

one more missing functionality: moban expects all templates to have existed before execution. In this case, one output of template engine become the input of another, this will cause exception for current moban.

@chfw
Copy link
Member

chfw commented Mar 11, 2019

And to truly respect the template file based sequence order( rather than template type based sequence order), two alternatives can be considered:

a. ignore moban engine optimization where if found that a data config or a template file is used across many templating actions, the file will be re-used across these actions.

b. provide higher level of abstraction os that render_to_files does not get invoked immediately but only generate an execution sequence, and such a sequence is given to a batch runner which does the action no matter which templating engine is involved. Then, we can consider include a in the batch runner so that render_to_files will drop the optimization and batch runner will try to re-implement the optimization.

@chfw
Copy link
Member

chfw commented Mar 11, 2019

Third alternative is to implement trailing spaces cleaner as post templating action.

- template: a.template.jj2
  output: filtered.output
  post: do_the_traing_spaces_filtering_please
  template_type: jinja2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
0.7.x
post-release
parity
Awaiting triage
Development

No branches or pull requests

2 participants