Summary: | TagsExternalDictionary does not work under Plack | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | abl, brendan, f.demians, julian.maurice, m.de.rooy, mtompset, veron |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 16444 | ||
Bug Blocks: | 16637 | ||
Attachments: |
Bug 16455: Remove the "Too late to run INIT block" from C4::Tags
Bug 16455: Remove the "Too late to run INIT block" from C4::Tags Bug 16455: Remove the "Too late to run INIT block" from C4::Tags Bug 16697: Translatability: Fix problem with isolated "'s"in request.tt |
Description
Jonathan Druart
2016-05-05 15:57:43 UTC
Created attachment 51247 [details] [review] Bug 16455: Remove the "Too late to run INIT block" from C4::Tags The pref TagsExternalDictionary is used to tell Lingua::Ispell to use an other dictionary, different from the default one (/usr/bin/ispell). To do so we need to set $Lingua::Ispell::path to the expected path. It's currently done in the INIT block. If you try to use C4::Tags, you will get the famous "Too late to run INIT block at C4/Tags.pm line 74." warning. Plack use the INIT block to load functions at run time, when we are using C4::Tags when hitting a pl script, the compilation phase is finished and it's "too late to run INIT block" from C4::Tags. I do not really know if it has an impact on the behavior of Lingua::Ispell (i.e. is the path redefined?), but I know that this INIT block is not executed when we want. Test plan: under Plack, - hit /cgi-bin/koha/opac-search.pl and confirm that the warning does no longer appears - Use another dictionnary (??), fill TagsExternalDictionary with its path and confirm that it is used by the tags approval system Oops... duplicated effort, but for different reasons. See bug 16637. I think adding the PerlDependencies.pm for Lingua::Ispell might be an idea. Otherwise you may get an explosion when you fill it in with the default value /usr/bin/ispell, because Lingua::Ispell isn't guaranteed to be installed. (In reply to M. Tompsett from comment #2) > Oops... duplicated effort, but for different reasons. See bug 16637. I think > adding the PerlDependencies.pm for Lingua::Ispell might be an idea. > Otherwise you may get an explosion when you fill it in with the default > value /usr/bin/ispell, because Lingua::Ispell isn't guaranteed to be > installed. I'd prefer to keep this one (especially because I have already submitted other patches using the "does not work under plack" pattern in commit msg). And maybe create another one for the missing deps. Created attachment 51970 [details] [review] Bug 16455: Remove the "Too late to run INIT block" from C4::Tags The pref TagsExternalDictionary is used to tell Lingua::Ispell to use an other dictionary, different from the default one (/usr/bin/ispell). To do so we need to set $Lingua::Ispell::path to the expected path. It's currently done in the INIT block. If you try to use C4::Tags, you will get the famous "Too late to run INIT block at C4/Tags.pm line 74." warning. Plack use the INIT block to load functions at run time, when we are using C4::Tags when hitting a pl script, the compilation phase is finished and it's "too late to run INIT block" from C4::Tags. I do not really know if it has an impact on the behavior of Lingua::Ispell (i.e. is the path redefined?), but I know that this INIT block is not executed when we want. Test plan: under Plack, - hit /cgi-bin/koha/opac-search.pl and confirm that the warning does no longer appears - Use another dictionnary (??), fill TagsExternalDictionary with its path and confirm that it is used by the tags approval system Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> QA: Claiming this one too now Created attachment 52051 [details] [review] Bug 16455: Remove the "Too late to run INIT block" from C4::Tags The pref TagsExternalDictionary is used to tell Lingua::Ispell to use an other dictionary, different from the default one (/usr/bin/ispell). To do so we need to set $Lingua::Ispell::path to the expected path. It's currently done in the INIT block. If you try to use C4::Tags, you will get the famous "Too late to run INIT block at C4/Tags.pm line 74." warning. Plack use the INIT block to load functions at run time, when we are using C4::Tags when hitting a pl script, the compilation phase is finished and it's "too late to run INIT block" from C4::Tags. I do not really know if it has an impact on the behavior of Lingua::Ispell (i.e. is the path redefined?), but I know that this INIT block is not executed when we want. Test plan: under Plack, - hit /cgi-bin/koha/opac-search.pl and confirm that the warning does no longer appears - Use another dictionnary (??), fill TagsExternalDictionary with its path and confirm that it is used by the tags approval system Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Comment on attachment 52051 [details] [review] Bug 16455: Remove the "Too late to run INIT block" from C4::Tags Review of attachment 52051 [details] [review]: ----------------------------------------------------------------- ::: C4/Tags.pm @@ +57,4 @@ > } > if ($ext_dict) { > require Lingua::Ispell; > + import Lingua::Ispell qw(spellcheck add_word_lc); Why is save_dictionary missing? (In reply to M. Tompsett from comment #7) > Comment on attachment 52051 [details] [review] [review] > Bug 16455: Remove the "Too late to run INIT block" from C4::Tags > > Review of attachment 52051 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/Tags.pm > @@ +57,4 @@ > > } > > if ($ext_dict) { > > require Lingua::Ispell; > > + import Lingua::Ispell qw(spellcheck add_word_lc); > > Why is save_dictionary missing? Is not used. At least I looked for it.. (In reply to Marcel de Rooy from comment #8) > (In reply to M. Tompsett from comment #7) > > Why is save_dictionary missing? > Is not used. At least I looked for it.. Okay. I went hunting further, and agree it isn't used. Just made applying the patch difficult. So I worked on 16637 without this, since there is no overlap in files. Pushed to Master - Should be in the November 2016 Release. Thanks Created attachment 52193 [details] [review] Bug 16697: Translatability: Fix problem with isolated "'s"in request.tt This patch fixes translation problems with tag-isolated 's in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt The patch fixes it at two places. In consequence, some other texts ares sigthly changed to make them look similar. To test: - Inspect code changes and verify that the new wording makes sense. Comment on attachment 52193 [details] [review] Bug 16697: Translatability: Fix problem with isolated "'s"in request.tt Sorry for the wrong attachment, I was distracted and mistaked. Pushed in 16.05. Will be in 16.05.01. Patch pushed to 3.22.x, will be in 3.22.8 |