Bug 39715 removed quotes around DataTables options. We should enforce it with a test to prevent further occurrences.
Created attachment 189994 [details] [review] Bug 41324: Add a xt test Patch from commit a307855
Created attachment 189995 [details] [review] Bug 41324: Tidy some kohaTable blocks - examples Commit using --no-verify to ease rebase
Created attachment 189996 [details] [review] Bug 41324: Tidy modified files Commit the tidiness perl misc/devel/tidy.pl $(git diff --name-only HEAD~1)
Patches for discussion. One last thing that is not tidy is the script tags from the .tt files. If script tags have TT tags, the prettier plugin cannot tidy them. I was trying to provide a solution to prevent regressions for bug 39715. I think this is great, but it will introduce conflicts. I am happy to continue, but I first need to know if people want that. The idea is to use JS variables instead of TT variables, and have the assignment in a first script block. example: ``` <script> // This cannot be tidy [% IF foo %] # do something [% ELSE %] let foo = 42; [% END %] </script> ``` => ``` <script> // This still cannot be tidy let foo = [% foo ? 1 : 0 %]; </script> <script> // But now this will be tidy automatically! if ( foo ) { # do something } else { let foo = 42; } </script> ``` On this bug report I will mainly focus on kohaTable, but it may actually impact whole template file. See also bug 41323 for the "remaining ones".
I think this looks great, and is very much in line with what I was trying to do with bugs like bug 23885.
To achieve this, one child bug report has been created per module. Here is the test plan for all of them. 1. Variable serialization In most of the scripts we need to use Template::Toolkit variable (Perl) for JavaScript logic. We are simply going to serialize them. eg. my js_var = [% my_tt_var | html %]; That might generate my js_var = ; if the TT variable is not defined. To prevent invalid JS code we are going to surround it with quotes: my js_var = "[% my_tt_var | html %]"; This might be a problem when we compare strings and integer (keep in mind when testing). When we want to generate a boolean we will use: const js_var = [% tt_var ? 1 : 0 | html %]; No need to quote. 2. Use of permissions and sysprefs See 41562. For permissions and sysprefs we are going to use a global variable that will be shared between the different script tags. That will avoid situations where a JS variable is erased later in a separate script tag. 3. "table_settings" variable There may be a problem with the "table_settings" variable. In some scripts we have several variables named "table_settings", that causes problem in DT init. To prevent that we are simply going to rename the variable to make it more specific.
Created attachment 191189 [details] [review] Bug 41324: Add a xt test Patch from commit 170433c
With all related bugs applied there is only one failure: koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt it's a tricky one, I will clean it later.
All this is more than just about kohaTable: it tidy all script tags in the templates with kohaTable.
The whole tree is available at https://gitlab.com/joubu/Koha/-/commits/bug_41324
Why do we tidy script tags? 1. Consistency The entire codebase is already tidy, except script tags. 2. Detect error early Running Prettier on JS code helps catch syntax errors and potential issues earlier 3. Clear separation of concerns Isolate TT and JS variables make code more robust and maintainable. This separation also paves the way for migrating JS logic to dedicated .js files 4. Predictable refactoring Cleaning up script tags in bulk prevents unexpected large diffs later. Without this the tidy script might suddenly reformat a script tag when the last TT tag is removed, confusing the author of the change