GNU bug report logs - #58136
[PATCH] ui: Improve sort order when searching package names.

Previous Next

Full log

View this message in rfc822 format

From: zimoun <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Cc: 58136 <at> debbugs.gnu.org
Subject: [bug#58136] [PATCH] ui: Improve sort order when searching package names.
Date: Mon, 17 Oct 2022 10:19:48 +0200

Hi Lars, Ludo,

In short, I miss why the initial patch with a minor tweak is not enough
for covering the corner cases. :-)


On lun., 17 oct. 2022 at 09:46, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> +++ b/guix/ui.scm
>> @@ -1623,10 +1623,23 @@ (define (relevance obj regexps metrics)
>>    (define (score regexp str)
>>      (fold-matches regexp str 0
>>                    (lambda (m score)
>> -                    (+ score
>> -                       (if (string=? (match:substring m) str)
>> -                           5             ;exact match
>> -                           1)))))
>> +                    (let* ((start (- (match:start m) 1))
>> +                           (end (match:end m))
>> +                           (left (if (>= start 0) (string-ref str start) #f))
>> +                           (right (if (< end (string-length str)) (string-ref str end) #f))
>> +                           (delimiter-classes '(Cc Cf Pd Pe Pf Pi Po Ps Sk Zs Zl Zp))
>> +                           (delim-left (or (member (and=> left char-general-category) delimiter-classes) (eq? left #f)))
>> +                           (delim-right (or (member (and=> right char-general-category) delimiter-classes) (eq? right #f))))
>> +                      (max score
>> +                        (cond
>> +                          ;; regexp is a full match for str.
>> +                          ((and (eq? left #f) (eq? right #f)) 4)
>> +                          ;; regexp matches a single word in str.
>> +                          ((and delim-left delim-right) 3)
>> +                          ;; regexp matches the beginning or end of a word in str.
>> +                          ((or delim-left delim-right) 2)
>> +                          ;; Everything else.
>> +                          (#t 1)))))))
>
> The intent is to have all regexps behave as if the user passed \<STR\>,
> is that right?  Would be nice to have a comment clarifying that above
> and perhaps making it a separate change?

All this appears to me overcomplicated.  Personally, I have to read it
many times to get the logic; while the initial patch was much clearer,
IMHO.

Other said, I am not convinced the complexity is worth the corner case.


The initial patch with the minor tweak I am proposing (maybe using
package-upstream-name*) appears to me enough for covering the corner
cases initially reported (as ggplot2).


>> @@ -1635,10 +1648,11 @@ (define (regexp->score regexp)
>>                  ((field . weight)
>>                   (match (field obj)
>>                     (#f  relevance)
>> +                   ('() relevance)
>>                     ((? string? str)
>> -                    (+ relevance (* (score-regexp str) weight)))
>> +                    (max relevance (* (score-regexp str) weight)))
>>                     ((lst ...)
>> -                    (+ relevance (* weight (apply + (map score-regexp lst)))))))))
>> +                    (max relevance (* weight (apply max (map score-regexp lst)))))))))
>
> Intuitively I would expect scores to add up, otherwise we’re kinda
> losing information; so I would not make this change.  WDYT?

I agree with Ludo that ’max’ is counterintuitive.


Well, could we list some examples (keyword and expectation)?  Because
the initial patch with a minor tweak LGTM and covers ggplot2, csv, and
some others.


Cheers,
simon

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.