[PATCH] org-agenda: Skip formatting if format string is ""

[PATCH] org-agenda: Skip formatting if format string is ""

[Samim Pezeshki]

[-- Attachment #1.1: Type: text/plain, Size: 503 bytes --]

Hi!

I had contributed a patch which was merged at
commit 90a9ee17410a4b6663f5957c653258e9f9418d7d .

This commit prevents having extra spaces when the TODO format string is an
empty string ("").  It was not working properly, with this patch now it
works correctly.

Attached is the screenshot of the bug, you see there is an extra space in
the place of the TODO keyword. The third heading (Meditate) has a TODO
keyword, and the org-agenda-todo-keyword-format is set to an empty string.


Thanks,
Samim

[-- Attachment #1.2: Type: text/html, Size: 1659 bytes --]

[-- Attachment #2: 0001-lisp-org-agenda.el-Fix-bug-in-org-agenda-highlight-t.patch --]
[-- Type: text/x-patch, Size: 1599 bytes --]

From 7e49286c49ab342f91504205fc72ac6a8d095a1f Mon Sep 17 00:00:00 2001
From: Samim Pezeshki <psamim@gmail.com>
Date: Sat, 5 Feb 2022 20:11:19 +0330
Subject: [PATCH] lisp/org-agenda.el: Fix bug in `org-agenda-highlight-todo'

* lisp/org-agenda.el (org-agenda-highlight-todo): Skip formatting
the to-do keyword when `org-agenda-todo-keyword-format' is the
empty string.

TINYCHANGE
---
 lisp/org-agenda.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 9bc44a56e..fea0675e7 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7634,13 +7634,13 @@ The optional argument TYPE tells the agenda type."
 	    (setq x
 		  (concat
 		   (substring x 0 (match-end 1))
-                   (unless (string= org-agenda-todo-keyword-format "")
-		     (format org-agenda-todo-keyword-format
-			     (match-string 2 x)))
-                   ;; Remove `display' property as the icon could leak
-		   ;; on the white space.
-		   (org-add-props " " (org-plist-delete (text-properties-at 0 x)
-                                                        'display))
+       (unless (string= org-agenda-todo-keyword-format "")
+         (format org-agenda-todo-keyword-format
+                             (match-string 2 x))
+         ;; Remove `display' property as the icon could leak
+         ;; on the white space.
+         (org-add-props " " (org-plist-delete (text-properties-at 0 x)
+                                              'display)))
                    (substring x (match-end 3)))))))
       x)))
 
-- 
2.35.1


[-- Attachment #3: 2022-02-05_20-29.png --]
[-- Type: image/png, Size: 6263 bytes --]

Re: [PATCH] org-agenda: Skip formatting if format string is ""

[Ihor Radchenko]

Samim Pezeshki <p.samim@gmail.com> writes:

> This commit prevents having extra spaces when the TODO format string is an
> empty string ("").  It was not working properly, with this patch now it
> works correctly.

Thanks for the patch!

>  		  (concat
>  		   (substring x 0 (match-end 1))
> ...
> +       (unless (string= org-agenda-todo-keyword-format "")
> +         (format org-agenda-todo-keyword-format
> +                             (match-string 2 x))
> +         ;; Remove `display' property as the icon could leak
> +         ;; on the white space.
> +         (org-add-props " " (org-plist-delete (text-properties-at 0 x)
> +                                              'display)))
>                     (substring x (match-end 3)))))))
>        x)))

Your patch will unconditionally hide todo keywords in agenda even when
org-agenda-todo-keyword-format is not empty. This will happen because
(unless ...) form will only return the last " ", but never the keyword.

You should better wrap the (org-add-props ...) with another unless
condition.

Best,
Ihor

Re: [PATCH] org-agenda: Skip formatting if format string is ""

[Samim Pezeshki]

[-- Attachment #1.1: Type: text/plain, Size: 1307 bytes --]

Thanks Ihor for the detailed explanation,
I updated the patch as you suggested.

On Sun, Feb 6, 2022 at 12:53 PM Ihor Radchenko <yantar92@gmail.com> wrote:

> Samim Pezeshki <p.samim@gmail.com> writes:
>
> > This commit prevents having extra spaces when the TODO format string is
> an
> > empty string ("").  It was not working properly, with this patch now it
> > works correctly.
>
> Thanks for the patch!
>
> >                 (concat
> >                  (substring x 0 (match-end 1))
> > ...
> > +       (unless (string= org-agenda-todo-keyword-format "")
> > +         (format org-agenda-todo-keyword-format
> > +                             (match-string 2 x))
> > +         ;; Remove `display' property as the icon could leak
> > +         ;; on the white space.
> > +         (org-add-props " " (org-plist-delete (text-properties-at 0 x)
> > +                                              'display)))
> >                     (substring x (match-end 3)))))))
> >        x)))
>
> Your patch will unconditionally hide todo keywords in agenda even when
> org-agenda-todo-keyword-format is not empty. This will happen because
> (unless ...) form will only return the last " ", but never the keyword.
>
> You should better wrap the (org-add-props ...) with another unless
> condition.
>
> Best,
> Ihor
>

[-- Attachment #1.2: Type: text/html, Size: 2128 bytes --]

[-- Attachment #2: 0001-lisp-org-agenda.el-Fix-bug-in-org-agenda-highlight-t.patch --]
[-- Type: text/x-patch, Size: 1733 bytes --]

From 2e90cac33c22cf8ea09f7f02e644df983b7ca0d1 Mon Sep 17 00:00:00 2001
From: Samim Pezeshki <psamim@gmail.com>
Date: Sat, 5 Feb 2022 20:11:19 +0330
Subject: [PATCH] lisp/org-agenda.el: Fix bug in `org-agenda-highlight-todo'

* lisp/org-agenda.el (org-agenda-highlight-todo): Skip formatting
the to-do keyword when `org-agenda-todo-keyword-format' is the
empty string.

TINYCHANGE
---
 lisp/org-agenda.el | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 9bc44a56e..3b6397b25 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7634,14 +7634,15 @@ The optional argument TYPE tells the agenda type."
 	    (setq x
 		  (concat
 		   (substring x 0 (match-end 1))
-                   (unless (string= org-agenda-todo-keyword-format "")
-		     (format org-agenda-todo-keyword-format
-			     (match-string 2 x)))
-                   ;; Remove `display' property as the icon could leak
-		   ;; on the white space.
-		   (org-add-props " " (org-plist-delete (text-properties-at 0 x)
-                                                        'display))
-                   (substring x (match-end 3)))))))
+       (unless (string= org-agenda-todo-keyword-format "")
+         (format org-agenda-todo-keyword-format
+                 (match-string 2 x)))
+       (unless (string= org-agenda-todo-keyword-format "")
+         ;; Remove `display' property as the icon could leak
+         ;; on the white space.
+         (org-add-props " " (org-plist-delete (text-properties-at 0 x)
+                                              'display)))
+       (substring x (match-end 3)))))))
       x)))
 
 (defsubst org-cmp-values (a b property)
-- 
2.35.1

Re: [PATCH] org-agenda: Skip formatting if format string is ""

[Samim Pezeshki]

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

Hi!

Is there anything I need to do for this to be merged?

Thanks

On Wed, Feb 9, 2022 at 10:44 PM Samim Pezeshki <p.samim@gmail.com> wrote:

> Thanks Ihor for the detailed explanation,
> I updated the patch as you suggested.
>
> On Sun, Feb 6, 2022 at 12:53 PM Ihor Radchenko <yantar92@gmail.com> wrote:
>
>> Samim Pezeshki <p.samim@gmail.com> writes:
>>
>> > This commit prevents having extra spaces when the TODO format string is
>> an
>> > empty string ("").  It was not working properly, with this patch now it
>> > works correctly.
>>
>> Thanks for the patch!
>>
>> >                 (concat
>> >                  (substring x 0 (match-end 1))
>> > ...
>> > +       (unless (string= org-agenda-todo-keyword-format "")
>> > +         (format org-agenda-todo-keyword-format
>> > +                             (match-string 2 x))
>> > +         ;; Remove `display' property as the icon could leak
>> > +         ;; on the white space.
>> > +         (org-add-props " " (org-plist-delete (text-properties-at 0 x)
>> > +                                              'display)))
>> >                     (substring x (match-end 3)))))))
>> >        x)))
>>
>> Your patch will unconditionally hide todo keywords in agenda even when
>> org-agenda-todo-keyword-format is not empty. This will happen because
>> (unless ...) form will only return the last " ", but never the keyword.
>>
>> You should better wrap the (org-add-props ...) with another unless
>> condition.
>>
>> Best,
>> Ihor
>>
>

[-- Attachment #2: Type: text/html, Size: 3084 bytes --]

Re: [PATCH] org-agenda: Skip formatting if format string is ""

[Ihor Radchenko]

Samim Pezeshki <p.samim@gmail.com> writes:

> Is there anything I need to do for this to be merged?

Applied.

I took a freedom to push your patch to bugfix amending the indentation
changes. (847e33230)

Note that your total contribution to Org is approaching 15 LOC - a legal
threshold for insignificant changes. If you want to contribute to Org
further, please consider signing the copyright papers. See
https://orgmode.org/worg/org-contribute.html#copyright for more details.

Best,
Ihor

[PATCH] org-agenda: Skip formatting if format string is ""

[Samim Pezeshki]

[-- Attachment #1.1: Type: text/plain, Size: 248 bytes --]

Hi!

This is my first patch.

The patch skips formatting TODO item in org-agenda when the format string
is "".
When the format string was "" the extra space would still be inserted which
was not needed when there is no TODO keyword.

Thanks,
Samim

[-- Attachment #1.2: Type: text/html, Size: 1214 bytes --]

[-- Attachment #2: 0001-org-agenda-Skip-formatting-if-format-string-is.patch --]
[-- Type: text/x-patch, Size: 1331 bytes --]

From dc3ba301219622bc7d324901a5f277024f8b55cc Mon Sep 17 00:00:00 2001
From: Samim Pezeshki <psamim@gmail.com>
Date: Sat, 8 May 2021 20:09:16 +0430
Subject: [PATCH] org-agenda: Skip formatting if format string is ""

When the format string was "" the extra space would still be inserted
which was not needed when there is no TODO keyword.
---
 lisp/org-agenda.el | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 85e201086..32ed19afc 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7147,12 +7147,13 @@ The optional argument TYPE tells the agenda type."
 	    (setq x
 		  (concat
 		   (substring x 0 (match-end 1))
-		   (format org-agenda-todo-keyword-format
-			   (match-string 2 x))
-		   ;; Remove `display' property as the icon could leak
-		   ;; on the white space.
-		   (org-add-props " " (org-plist-delete (text-properties-at 0 x)
-							'display))
+       (when (not (eq org-agenda-todo-keyword-format ""))
+		    (format org-agenda-todo-keyword-format
+			    (match-string 2 x))
+		    ;; Remove `display' property as the icon could leak
+		    ;; on the white space.
+		    (org-add-props " " (org-plist-delete (text-properties-at 0 x)
+			 				 'display)))
 		   (substring x (match-end 3)))))))
       x)))
 
-- 
2.31.1

Re: [PATCH] org-agenda: Skip formatting if format string is ""

[Bastien]

Hi Samim,

Samim Pezeshki <p.samim@gmail.com> writes:

> This is my first patch.

Thank you!

> The patch skips formatting TODO item in org-agenda when the format
> string is "". 
> When the format string was "" the extra space would still be inserted
> which was not needed when there is no TODO keyword.

Applied in the maint branch as 06c064e97 with some minor modifications
and a proper changelog entry in the commit message.

See https://orgmode.org/worg/org-contribute.html#commit-messages

Thanks,

-- 
 Bastien