Summary: | Add auto-completion on auth_finder | ||
---|---|---|---|
Product: | Koha | Reporter: | Matthias Meusburger <matthias.meusburger> |
Component: | Searching | Assignee: | Matthias Meusburger <matthias.meusburger> |
Status: | CLOSED FIXED | QA Contact: | Ian Walls <koha.sekjal> |
Severity: | enhancement | ||
Priority: | P3 | CC: | chris, jcamins, jonathan.druart, paul.poulain |
Version: | 3.8 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Bug Depends on: | |||
Bug Blocks: | 8875 | ||
Attachments: |
proposed patch
Bug 7400: Add auto-completion on auth_finder Proposed patch to pass qa Signed-off patch proposed patch Bug 7400: Add auto-completion on auth_finder |
Description
Matthias Meusburger
2012-01-04 14:11:28 UTC
Created attachment 7036 [details] [review] proposed patch Created attachment 7082 [details] [review] Bug 7400: Add auto-completion on auth_finder While typing an authority, will automatically propose authorities (similar to autocompletion for patron search if activated) Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Tested searching for authorities with and without autocomplete. Note that this is most useful when used in the "Main entry" box instead of the "Main entry ($a only)" box. QA comment: You add a new nsb_clean function. This function is already present in cataloguing/ysearch.pl. Isn't better to factorize it in a C4 module ? Why do you comment the "use warning" line ? You can now use Modern::Perl instead of use strict; use warnings; :) Created attachment 7944 [details] [review] Proposed patch to pass qa The nsb_clean function has been added to C4::Charset and removed from the two ysearch.pl Indeed, "use warnings" had no reason to be commented. It has been removed and replaced by "use Modern::Perl" in both ysearch.pl's. Created attachment 7993 [details] [review] Signed-off patch Uses the Switch module, which I believe is considered deprecated for 5.12+. See http://www.perlmonks.org/?node_id=931583. Since 5.10, there is a built-in switch operator, then given/when syntax. Since Koha is now based on 5.10 minimum, we can safely depend on this. See http://perldoc.perl.org/perlsyn.html#Switch-statements. Marking Failed QA until the dependency on Switch is removed. Created attachment 8120 [details] [review] proposed patch Proposed patch using "given" instead of "switch". Created attachment 8142 [details] [review] Bug 7400: Add auto-completion on auth_finder While typing an authority, will automatically propose authorities (similar to autocompletion for patron search if activated) Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Tested searching for authorities with and without autocomplete. Note that this is most useful when used in the "Main entry" box instead of the "Main entry ($a only)" box. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Corrected tabs to spaces in auth-finder-search.inc while resolving merge conflict. Now that the dependency on Switch has been removed, this code looks good to me. Moves nsb_clean to C4/Charset.pm, which increases usability. Marking as Passed QA patch pushed, but i'm wondering if we should not have a syspref to activate/desactivate all auto-completion things. This one is not managed by any syspref, couldn't it be a performance problem for some libraries ? (In reply to comment #11) > patch pushed, but i'm wondering if we should not have a syspref to > activate/desactivate all auto-completion things. > This one is not managed by any syspref, couldn't it be a performance problem > for some libraries ? I agree first thing I do is disable auto completes as it works the db hard. |