search

edgewall / genshi

Python toolkit for generation of output for the web
http://genshi.edgewall.org
Other
86 stars 35 forks source link

Do not merge sub directives if they have not been changed #53

Closed cedk closed 3 months ago

cedk commented 2 years ago

When extracting new directives only new sub directives must be merged in the parent sub directive.

Fixes #48

FelixSchwarz commented 2 years ago

Thank you very much for your contribution. As the test suite is happy I guess we can merge that. However the new condition seems to be pretty complex and I can't figure out what it actually does. Any chance we can ease understanding that piece of code with some local variables (= good naming) or a comment?

cedk commented 2 years ago

Indeed it is testing that the new directives have been added to the substream by the extra directive. I do not know how you are naming usually the content of a substream?

cedk commented 2 years ago

@FelixSchwarz Is the comment enough? It will be great if we could have soon a release with this fix.

FelixSchwarz commented 2 years ago

@cedk I guess the comment is ok - the code itself is complex but that's the way it is.

Unfortunately I have almost zero free time in December so @hodgestar could handle/merge this issue?

zipus commented 2 years ago

Unfortunately I have almost zero free time in December so @hodgestar could handle/merge this issue?

Could anyone find some time to merge?

cedk commented 2 years ago

It is too bad this did not reach 0.7.6. Is there something I can do to not miss the next release?

hodgestar commented 2 years ago

@cedk I'm sorry this didn't get merged, but I didn't want to delay the release. Now that I've migrated the release process to Github Actions, I think the solution is to just do another release as soon as the current PRs are dealt with.