emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: "Sławomir Grochowski" <slawomir.grochowski@gmail.com>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: emacs-orgmode@gnu.org
Subject: Re: [BUG] colview.el regexp - capture operator when title is empty
Date: Sun, 25 Aug 2024 14:35:20 +0200	[thread overview]
Message-ID: <875xrobx13.fsf@gmail.com> (raw)
In-Reply-To: <87zfp6cmbh.fsf@gmail.com>

I tried to describe the problem better as well as my thought process:

The `org-columns-compile-format' function has a bug in regexp.
When an empty parentheses `()` is in the 'column format string' -
and it always is when user do not provide a column title - then the
regexp can't capture the operator which is in curly brackets `{}`.

Try yourself with code below:
#+begin_src elisp
(org-columns-compile-format"%ITEM(){operator}") 
; return => (("ITEM" "ITEM" nil nil nil)) 
; should be => (("ITEM" "ITEM" nil operator nil)) 
#+end_src

I'm not very skilled in regex, so following Ihor's advice I decided to
write the regex in the form of 'rx' which is more readable. Below is
the modified function:

#+begin_src elisp
  (defun org-columns-compile-format-rx (fmt)
    "Turn a column format string FMT into an alist of specifications.

    The alist has one entry for each column in the format.  The elements of
    that list are:
    property    the property name, as an upper-case string
    title       the title field for the columns, as a string
    width       the column width in characters, can be nil for automatic width
    operator    the summary operator, as a string, or nil
    printf      a printf format for computed values, as a string, or nil

    This function updates `org-columns-current-fmt-compiled'."
    (setq org-columns-current-fmt-compiled nil)
    (let ((start 0))
      (while (string-match
              (rx "%"
                  (optional (group (+ digit)))
                  (group (one-or-more (in alnum "_-")))
                  (optional "(" (group (one-or-more (not (any ")")))) ")")
                  (optional "{" (group (one-or-more (not (any "}")))) "}")
                  (zero-or-more space))
    	    fmt start)
        (setq start (match-end 0))
        (let* ((width (and (match-end 1) (string-to-number (match-string 1 fmt))))
    	     (prop (match-string-no-properties 2 fmt))
    	     (title (or (match-string-no-properties 3 fmt) prop))
    	     (operator (match-string-no-properties 4 fmt)))
    	(push (if (not operator) (list (upcase prop) title width nil nil)
    		(let (printf)
    		  (when (string-match ";" operator)
    		    (setq printf (substring operator (match-end 0)))
    		    (setq operator (substring operator 0 (match-beginning 0))))
    		  (list (upcase prop) title width operator printf)))
    	      org-columns-current-fmt-compiled)))
      (setq org-columns-current-fmt-compiled
    	  (nreverse org-columns-current-fmt-compiled))))
#+end_src

I am checking if in this particular case it returns the same result 
as 'org-columns-compile-format': 

#+begin_src elisp
 (org-columns-compile-format-rx "%ITEM(){operator}") ;; => (("ITEM" "ITEM" nil nil nil))
#+end_src

#+begin_src elisp
  (equal 
   (org-columns-compile-format "%ITEM(){operator}") 
   (org-columns-compile-format-rx "%ITEM(){operator}")) ;; => t
#+end_src

Yes, in this particular case it works the same. To make it easier to
capture the error, I only take a piece of code responsible for the
regexp:

#+begin_src elisp
(let ((text "%25ITEM(){operator}")
        (pattern (rx "%"
                  (optional (group (+ digit)))
                  (group (one-or-more (in alnum "_-")))
                  (optional "(" (group (one-or-more (not (any ")")))) ")")
                  (optional "{" (group (one-or-more (not (any "}")))) "}")
                  (zero-or-more space))))
    (if (string-match pattern text)
        (mapcar (lambda (i) (match-string i text))
                (number-sequence 0 (/ (length (match-data)) 2)))))
;=> ("%25ITEM" "25" "ITEM" nil)
#+end_src

Through trial and error, I came to a solution that provides correct
results. I changed the expression 'one-or-more' to 'zero-or-more' for
'title' and 'operator':

#+begin_src elisp
(let ((text "%25ITEM(){operator}")
        (pattern (rx "%"
                  (optional (group (+ digit)))
                  (group (one-or-more (in alnum "_-")))
                  (optional "(" (group (zero-or-more (not (any ")")))) ")")
                  (optional "{" (group (zero-or-more (not (any "}")))) "}")
                  (zero-or-more space))))
    (if (string-match pattern text)
        (mapcar (lambda (i) (match-string i text))
                (number-sequence 0 (/ (length (match-data)) 2)))))
;=> ("%25ITEM(){operator}" "25" "ITEM" "" "operator" nil)
#+end_src

This fixed a problem but also changed the return value. Now, when we
have empty parentheses '()', it will return an empty string
instead of null.

Therefore, in the function `org-columns-compile-format`, I added calls
to `org-string-nw-p` for the variables 'title' and 'operator'.

Additionally, I think that adding empty parentheses '()' should be removed
when we do not specify a 'column title'. Because it does not provide
any value. So I added another call to `org-string-nw-p' in function
`org-columns-new'.

Patch below:
<#part type="text/x-diff" filename="~/.emacs.d/straight/repos/org/0001-lisp-org-colview.el-org-columns-compile-format-regex.patch" disposition=inline>
<#/part>

Regards,
-- 
Slawomir Grochowski


  reply	other threads:[~2024-08-25 12:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19 10:45 [BUG] colview.el regexp - capture operator when title is empty Sławomir Grochowski
2024-08-20 18:21 ` Ihor Radchenko
2024-08-21  8:27   ` Sławomir Grochowski
2024-08-25 12:35     ` Sławomir Grochowski [this message]
2024-08-25 12:39       ` Ihor Radchenko
2024-08-25 12:59         ` Sławomir Grochowski
2024-08-31 13:34           ` Ihor Radchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=875xrobx13.fsf@gmail.com \
    --to=slawomir.grochowski@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=yantar92@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).