Re: [BUG] colview.el regexp - capture operator when title is empty

[Sławomir Grochowski]

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