**Tidy all the things** ([bug 38664](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38664)) We will tidy the whole code base: Perl (.pm, .pl, .t), Javascript (.js, .ts, .vue) and Template::Toolkit (.tt, .inc) files. A `tidy.pl` script will be provided to easily tidy the files. **Changelog** 2025-02-10 Agreed to remove "semi: false" for .vue files 2025-02-06 Opened bug 39059 (Do not use "semi: false" for .vue files) for discussion 2025-02-03 qa-test-tools (qa script) and koha-testing-docker (git hook) have a merge request ready (see below) 2025-01-29 @koha-community/prettier-plugin-template-toolkit is published! 2025-01-27 JS code within script tags is now indented (if no TT tag exist)! 2025-01-21 **Bug 38718 ready for testing**! 2025-01-21 **Bug 38714 ready for ~~testing~~ QA**! 2025-01-14 All Koha tests are passing on top of bug_38664_tidy! 2025-01-13 All tests for the prettier plugin are passing 2025-01-13 **Bug 38713 ready for ~~testing~~ QA**! --> **Pushed!** 2025-01-12 The tidy script is running successfully on all the templates (staff+opac) 2025-01-09 Remote branch with a "tidy everything" commit is pushed to a [separate branch](https://gitlab.com/joubu/Koha/-/commits/bug_38664_tidy) 2025-01-09 Staff interface done! 2025-01-08 The tidy script (misc/devel/tidy.pl)! 2024-12-18 The auto rebase script (misc/devel/auto_rebase.pl)! 2024-12-17 Everything is WIP/POC so far, nothing is ready for inclusion (update on MatterMost) 2024-12-15 Starting to investigate 2024-12-10 Opened the bug report and started the discussion The code is on a [gitlab branch](https://gitlab.com/joubu/Koha/-/commits/bug_38664). **Perl** It will be easy to run perltidy on the whole codebase. We will need a trivial xt test (using Test::PerlTidy), a QA test and a pre-commit git hook. **JavaScript** Easy as well, we do that already actually, the existing tests will need adjustment but it will be the easiest part. **Templates** That's the hard part. Nothing exists yet. I have hijacked an existing prettier plugin, to make it work with our template files. I pushed that to a gitlab branch: https://gitlab.com/joubu/prettier-plugin-jinja-template It will catch incorrect structures, missing closing tags (TT or html), etc. And keep everything tidy by fixing white-spaces and indentation levels. **The Auto-rebase script** This script automates the rebasing of a Git branch onto a target branch that contains the tidy version of the codebase. It rebases up to before the first commit of the tidy commits then applies the commits from the source branch, tidy this version and commits. Finally the rebase will continue. The source branch is the branch that is currently checked out. [See it in action](https://cloud.joetka.eu/s/SK7BjFRSggLt6fK/download/auto-rebase-2025-01-15_12.51.11.mkv)! **The tidy script** Located in `misc/devel/tidy.pl` it can be run without any options to tidy the whole codebase (files in the git index). You can also pass a list of files, or run only on a filetype (see the `--help`) **The different bug reports** Order of inclusion: * Bug 38713 - [OMNIBUS] Incorrect HTML structures (Pushed) Correct the HTML structure (wrong or missing tags, etc.) We might want/need to backport most of those changes. * Bug 38714 - [OMNIBUS] Adjust templates for prettier Adjust our template logic so that prettier can indent and tidy the templates. We need to make sure the HTML tags are not opened twice. For instance the following code is not correct: ``` [% IF selected %] <option value="" selected> [% ELSE %] <option value=""> [% END %] [% option_value | html %] </option> ``` A big part of the changes here consists in simple adjustements to the templates. But in some cases it's not trivial. After this change, all the template files should pass the tidy step. To confirm this you need to run the prettier plugin, see bug [38714 comment 3](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38714#c3). * Bug 38718 - main container wrapper Reduce the indentation level, so that we don't have very long lines. Also it will help to centralize the `main` and `aside`, as well as the divs of the top level containers. * Bug 38664 - Tidy the whole codebase **Outside of Koha** The prettier plugin needs to be cleaned, tests needs to be adjusted. The project needs to be renamed and published to have our own NPM package. **Limitation** * We may not support tidying of the `script` tags from template files. * Use of split("\|") in a loop (templates) is problematic. But it can be easily workarounded. See: ```diff - [% IF ITEM_RESULT.uri.split(' \| ').size > 1 %] - [% FOREACH uri IN ITEM_RESULT.uri.split(' \| ') %] + [% SET uris = ITEM_RESULT.uri.split(' \| ') %] + [% IF uris.size > 1 %] + [% FOREACH uri IN uris %] ``` * The translation process * Confirm that the changes make sense (`gulp po:update --lang LANG` before and after the "tidy all" patch) ``` - <option value="r" selected="selected">Remote-sensing image</option> + <option value="r" selected="selected"> + Remote-sensing image + </option> ``` => Generate ``` +#: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/marc21_field_007.tt:89 +#, fuzzy, c-format +msgid "Remote-sensing image " +msgstr "Image de télédétection" ``` Added a "WIP trim msgid" commit, and it actually fixes some problem with the current script: ``` +#. For the first occurrence, #. SCRIPT -#: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/marc21_field_007.tt:114 +#: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/marc21_field_007.tt:139 msgid "b- Large print" msgstr "b- Gros caractères" -#. SCRIPT -#: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/marc21_field_007.tt:114 -msgid "b- Large print " -msgstr "b- Gros caractères " ``` Is that enough? No it was not, we were loosing important spaces in some places. SOLUTION - we modify prettier's default behaviour: https://prettier.io/docs/en/options.html#html-whitespace-sensitivity Using "htmlWhitespaceSensitivity: 'strict'" in `.prettierrc.js` fixes the problem by splitting the tags themselves ``` + [% IF m.version == ( mm.version ) %] + <ul + ><li><strong>Mentor:</strong> [% INCLUDE person p=mm %]</li></ul + > + [% END %] ``` Confirm that those changes are correct: `git diff|grep -P "^(\+|-)msgid"|sed -s "s/.//"|sort|uniq` For example: ``` msgid "%s There are no received orders. %s " msgid "%sThere are no received orders.%s " msgid "%s This account cannot view requested patron information. %s " msgid "%sThis account cannot view requested patron information. %s " msgid "%s This record has no items. %s " msgid "%sThis record has no items.%s " msgid "Suggested by: %s%s, %s %s (" msgid "Suggested by: %s%s, %s%s (" msgid "%s Waiting at %s %s until %s %s " msgid "%s Waiting at %s %suntil %s%s " msgid "%s Waiting here %s until %s %s " msgid "%s Waiting here %s until %s%s. " msgid "%s Waiting here %suntil %s%s " msgid "%s Waiting here %suntil %s%s. " msgid "%s Yes%s , " msgid "%s Yes%s, " msgid "%s Yes %s No %s" ``` **Todo list** * ~~Start the discussion~~ * ~~Find a way to tidy Template::Toolkit files~~ * [DONE/NSO] Fix all the templates - 38713 and 38714 * [DONE/NSO] Remove the unecessary indentation levels using `main-container.inc` (bug 38718) * Confirm that HTML1 is no longer valid (Really?? see bug 38720) * Remove xt/tt_valid.t * [DONE] Test the translation process (see above) * Prettier adds missing closing tag at the end of the document (?) Try to find when this is a problem. There are certainly occurrences of new closing tags added at the end of the templates that must be flagged to ignore. * [DONE] Run the tidy script on everything and request for testing * Investigate the removal of whitespace (could be a problem), see also previous bullet about the translation proces * [DONE] Adjust the prettier plugin * Remove blank line added after the `script` tags everytime the script is run (expected to be fixed by 'script and style tags capture succeeding whitespace' but is not) * [DONE] Fix tests * [DONE] Rename * [DONE] Publish * [DONE] Remove hard-coded options from tidy.pl and use the ones from `.prettierrc.js` * [DONE ] QA checks - see https://gitlab.com/koha-community/qa-test-tools/-/issues/90 * Remove existing tidy checks * Add new ones (use misc/devel/tidy.pl and compare outputs?) * Remove hard-coded options and use the ones from `.prettierrc.js` * [DONE] git hooks - see https://gitlab.com/koha-community/koha-testing-docker/-/issues/475 * [DONE] xt tests (if not too slow to run) * [DONE] Deal with script tags within .tt (@paulderscheid is going to look into it, see last commit of my prettier-plugin-jinja-template if interested: "WIP - format script tags") * [LATER] Reintroduce xt/tt_valid.t See [bug 38720 comment 4](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38720#c4). * [DONE] Configure .git-blame-ignore-revs (see `git blame --ignore-revs-file .git-blame-ignore-revs`) - See [bug 39096](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39096) * [DONE] Ignore "Koha Development Team" from the release-tools script * [LATER] Follow-up bugs * [Bug 39084](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39084) - Unnecessary spaces added (actually preserved) by the tidy all work * [Bug 39085](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39085) - No space preserved incorrecly after END * [Bug 39086](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39086) - Lines incorrectly split * [Bug 39087](https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39087) - Some files are uglier when tidy * Deal with new dependency (Test::PerlTidy) - it's going to be installed in ktd using cpanm as a temporary solution (ping @mtj)
{}