[PATCH] Faulty logic in org-cmp-tag/alpha

[PATCH] Faulty logic in org-cmp-tag/alpha

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

Hi all,

there is a problem in this kind of logic:

    (cond ((not ta) +1)
      ((not tb) -1)
      ((string-lessp ta tb) -1)
      ((string-lessp tb ta) +1))))

in that when both ta and tb are nil then they are arbitrarily sorted.
Since the agenda sorting strategy is lexicographic this logic
virtually invalidates any strategy that puts tag or alpha first, v.g.
'(tag-up priority-down).

I've attached a patch returning nil when both ta and tb are nil so as
to fallback to the next sorting criterion without favoring lhs nor
rhs.

AFAICS this only affects tag and alpha comparators.

Best regards
--
Carlos

[-- Attachment #2: 0001-Compare-nil-nil-as-nil-so-as-not-to-change-lexicogra.patch --]
[-- Type: text/x-patch, Size: 1327 bytes --]

From 35396182b465bd67c72d776709eee5e0041d96c7 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Sat, 9 Mar 2019 17:49:52 -0300
Subject: [PATCH] Compare nil-nil as nil so as not to change lexicographic
 order

* lisp/org-agenda.el (org-cmp-alpha/tag): don't favor a particular
  ordering when both lhs and rhs are nil.
---
 lisp/org-agenda.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 13bbae966..c9bea4465 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7003,7 +7003,8 @@ The optional argument TYPE tells the agenda type."
 				  "\\([ \t]*\\[[a-zA-Z0-9]\\]\\)? *") tb)
 	(setq tb (substring tb (match-end 0))))
       (setq tb (downcase tb)))
-    (cond ((not ta) +1)
+    (cond ((not (or ta tb)) nil)
+	  ((not ta) +1)
 	  ((not tb) -1)
 	  ((string-lessp ta tb) -1)
 	  ((string-lessp tb ta) +1))))
@@ -7012,7 +7013,8 @@ The optional argument TYPE tells the agenda type."
   "Compare the string values of the first tags of A and B."
   (let ((ta (car (last (get-text-property 1 'tags a))))
 	(tb (car (last (get-text-property 1 'tags b)))))
-    (cond ((not ta) +1)
+    (cond ((not (or ta tb)) nil)
+	  ((not ta) +1)
 	  ((not tb) -1)
 	  ((string-lessp ta tb) -1)
 	  ((string-lessp tb ta) +1))))
-- 
2.21.0

Re: [PATCH] Faulty logic in org-cmp-tag/alpha

[Nicolas Goaziou]

Hello,

Carlos Pita <carlosjosepita@gmail.com> writes:

> Hi all,
>
> there is a problem in this kind of logic:
>
>     (cond ((not ta) +1)
>       ((not tb) -1)
>       ((string-lessp ta tb) -1)
>       ((string-lessp tb ta) +1))))
>
> in that when both ta and tb are nil then they are arbitrarily sorted.
> Since the agenda sorting strategy is lexicographic this logic
> virtually invalidates any strategy that puts tag or alpha first, v.g.
> '(tag-up priority-down).
>
> I've attached a patch returning nil when both ta and tb are nil so as
> to fallback to the next sorting criterion without favoring lhs nor
> rhs.

Applied. Thank you.

Regards,

-- 
Nicolas Goaziou