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.
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