[Bug 39353] New: Tidy - Do not empty template files
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Bug ID: 39353 Summary: Tidy - Do not empty template files Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 38664 See the description of the problem - https://gitlab.com/koha-community/koha-testing-docker/-/issues/483 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38664 [Bug 38664] Tidy the whole codebase -- 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=39353 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 179381 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179381&action=edit Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing:
From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
=> The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?) -- 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=39353 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- I don't think this fix is ideal and it would be better to fix the origin of the problem (either a potential prettier bug, and/or the possibility to have a single node_modules) NSO/In discussion. -- 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=39353 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@gmail.com |ity.org | -- 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=39353 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #3 from Victor Grousset/tuxayo <victor@tuxayo.net> --- *** Bug 39352 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #179381|0 |1 is obsolete| | --- Comment #4 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 179386 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179386&action=edit Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing:
From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
=> The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?) Test plan: 1. Follow this from step 1 to 14 https://gitlab.com/koha-community/koha-testing-docker/-/issues/483 2. Apply patch 3. misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt It should print the new error message 4. Make a change to a .tt file, commit 5. There will be a message about and issue with prettier. But it will still commit. It's expect. 6. Inspect and see that the commit has the changes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #179386|0 |1 is obsolete| | --- Comment #5 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 179387 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179387&action=edit Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing:
From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
=> The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?) Test plan: 1. Follow this from step 1 to 14 https://gitlab.com/koha-community/koha-testing-docker/-/issues/483 2. Apply patch 3. misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt It should print the new error message 4. Make a change to a .tt file, commit 5. There will be a message about and issue with prettier. But it will still commit. It's expect. 6. Inspect and see that the commit has the changes Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #6 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works! :) Added test plan. Tested misc/devel/tidy.pl with a .inc file to see that it's handled as the .tt -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 --- Comment #7 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Comment on attachment 179387 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179387 Bug 39353: Prevent tidy.pl to empty template files Review of attachment 179387: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=39353&attachment=179387) ----------------------------------------------------------------- Should severity be raised to critical at least? ::: misc/devel/tidy.pl @@ +203,5 @@
$content =~ s#\n*( *)</script>\n*#\n$1</script>\n#g; $content =~ s#(\[%\s*SWITCH[^\]]*\]\n)\n#$1#g;
+ unless ($content) { + return ( 0, "Something went wrong, Prettier generated an empty file.", [], [], [] );
Maybe add a bit to the message: "Something went wrong, Prettier generated an empty file. Original file was kept." So the output after a commit doesn't cast doubt about corruption. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |emily.lamancusa@montgomeryc |y.org |ountymd.gov CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- Note that https://gitlab.com/koha-community/koha-testing-docker/-/issues/485 is about removing one of the node_modules directory. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #179387|0 |1 is obsolete| | --- Comment #9 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 179480 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179480&action=edit Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing:
From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
=> The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?) Test plan: 1. Follow this from step 1 to 14 https://gitlab.com/koha-community/koha-testing-docker/-/issues/483 2. Apply patch 3. misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt It should print the new error message 4. Make a change to a .tt file, commit 5. There will be a message about and issue with prettier. But it will still commit. It's expect. 6. Inspect and see that the commit has the changes Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 --- Comment #10 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 179481 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179481&action=edit Bug 39353: (QA follow-up) Clarify error message Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #11 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- I agree it's better to fix the origin of the problem, but even once we do, it's still nice to have this error handling in place in case a similar problem comes up again in the future. This is a straightforward change and it works to prevent data loss until we're able to prevent the underlying problem. Passing QA, and a big thank you to everyone who collaborated on this one! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |25.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #13 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 38664 not in 24.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED CC| |david@davidnind.com Resolution|--- |FIXED --- Comment #14 from David Nind <david@davidnind.com> --- No updates to the manual required. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39353 Bug 39353 depends on bug 38664, which changed state. Bug 38664 Summary: Tidy the whole codebase https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38664 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org