[Bug 20657] New: Globally enable PRE_CHOMP and POST_CHOMP in Template Toolkit configuration
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 Bug ID: 20657 Summary: Globally enable PRE_CHOMP and POST_CHOMP in Template Toolkit configuration Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: oleonard@myacpl.org QA Contact: testopia@bugs.koha-community.org As it currently stands, the HTML output by Template Toolkit includes whitespace anywhere there was a template tag. For example, this is a portion of the generated source of the staff client home page: 1 2 3 4 <!DOCTYPE html> 5 <!-- TEMPLATE FILE: intranet-main.tt --> 6 7 8 9 10 <html lang="en"> 11 <head> 12 13 <title>Koha staff client</title> Developer tools report: 26.69 KB / 5.83 KB transferred In contrast, this what the same content looks like with PRE_CHOMP and POST_CHOMP enabled: 1 <!DOCTYPE html> 2 <!-- TEMPLATE FILE: intranet-main.tt --> 3 <html lang="en"><head> 4 <title>Koha staff client</title> Developer tools report: 23.29 KB / 5.65 KB transferred In the samples I looked at this change did not make the HTML source significantly more difficult to read. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I've suggested that yesterday: diff --git a/C4/Templates.pm b/C4/Templates.pm index 789495f3fae..7f7b36e3dc6 100644 --- a/C4/Templates.pm +++ b/C4/Templates.pm @@ -72,6 +72,9 @@ sub new { INCLUDE_PATH => \@includes, FILTERS => {}, ENCODING => 'UTF-8', + PRE_CHOMP => 3, + POST_CHOMP => 3, + TRIM => 1, } ) or die Template->error(); my $self = { @@ -126,6 +129,8 @@ sub output { binmode( STDOUT, ":encoding(UTF-8)" ); $template->process( $self->filename, $vars, \$data ) || die "Template process failed: ", $template->error(); + + $data =~ tr/ //s; return $data; } -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #2 from Julian Maurice <julian.maurice@biblibre.com> --- + $data =~ tr/ //s; This would squash *every* sequences of space into a single space, right ? I'm not sure if it's appropriate for every template (sometimes spaces have meaning, like in some MARC fields, or we want them, like in <pre></pre> blocks, ...) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Julian Maurice from comment #2)
+ $data =~ tr/ //s;
This would squash *every* sequences of space into a single space, right ? I'm not sure if it's appropriate for every template (sometimes spaces have meaning, like in some MARC fields, or we want them, like in <pre></pre> blocks, ...)
Yes, agreed, we should go with pre/post chomp first. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |Needs Signoff CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 148167 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148167&action=edit Bug 20657: Enable PRE_CHOMP, POST_CHOMP and TRIM by default it TT Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Submitting to get the ball rolling. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- For reference to the values picked by Jonathan on his proposal: http://template-toolkit.org/docs/manual/Config.html#section_PRE_CHOMP_POST_C... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Two issues I noticed on a quick test: 1 - Holdings tab on staff details is collapsed after this, you have to click to open/view 2 - Messages with spaces are affected by this: In transit fromCentervilletoSpringfieldsince03/09/2023 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Nick Clemens from comment #7)
Two issues I noticed on a quick test: 1 - Holdings tab on staff details is collapsed after this, you have to click to open/view 2 - Messages with spaces are affected by this: In transit fromCentervilletoSpringfieldsince03/09/2023
Yes, we would need to use this syntax on special cases [%+ +%] or so -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I faced some challenges with chomping in bug 31028.. this patch kind of moves the problem really. We've historically not been very good at being explicit about chomping flags in our templates and I've often found myself adding '-' to the opening or closing tags of template markers. This patch changes the default from 'no chomp' to 'chomp always'.. I think this is an improvement given our general use.. but we will likely need to explicitly disable the chomping in a number of cases to keep intended whitespace as Nick spotted. It also might be nice to clean up all uses of `-` as that's now effectively the detault. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think this is probably a case of push early and fix occurences we spot in the wild quickly after.. it's going to be hard to test all pages to fix things inline here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- We do, however, also allow the use of our includes within notices.. we could end up "fixing" includes for this and breaking notices that use them.. perhaps we should also make this change in C4::Letters? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The translation process also removes spaces in some cases, so we'd need to test really well using translated templates too. I agree with Martin about the early push. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Perhaps this is another one to go for before hackfest and use some hackfest time to work through tidying up.? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #13)
Perhaps this is another one to go for before hackfest and use some hackfest time to work through tidying up.?
Should we change this setting for notices? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #15 from Lucas Gass <lucas@bywatersolutions.com> --- Is this ready for testing now? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #16 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 163235 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163235&action=edit Bug 20657: Remove explicit chomp in patronfields.inc -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #17 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 163239 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163239&action=edit Bug 20657: Explicit + in class We need a script to catch those ones -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #18 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 163240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163240&action=edit Bug 20657: Explicit + for TT vars in sentences If inside div and span? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #19 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 163241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163241&action=edit Bug 20657: Add non-breaking space in alphabet list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #20 from Jonathan Druart <jonathan.druart@gmail.com> --- We are clearly not ready for that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 --- Comment #21 from Jonathan Druart <jonathan.druart@gmail.com> --- I will address the original issue (comment 0) on bug 36333. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20657 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org