emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Faulty logic in org-cmp-tag/alpha
@ 2019-03-09 20:55 Carlos Pita
  2019-03-10 10:15 ` Nicolas Goaziou
  0 siblings, 1 reply; 2+ messages in thread
From: Carlos Pita @ 2019-03-09 20:55 UTC (permalink / raw)
  To: emacs-orgmode

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] Faulty logic in org-cmp-tag/alpha
  2019-03-09 20:55 [PATCH] Faulty logic in org-cmp-tag/alpha Carlos Pita
@ 2019-03-10 10:15 ` Nicolas Goaziou
  0 siblings, 0 replies; 2+ messages in thread
From: Nicolas Goaziou @ 2019-03-10 10:15 UTC (permalink / raw)
  To: Carlos Pita; +Cc: emacs-orgmode

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-03-10 10:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-09 20:55 [PATCH] Faulty logic in org-cmp-tag/alpha Carlos Pita
2019-03-10 10:15 ` Nicolas Goaziou

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).