Earl Chase <[email protected]> writes:

> This was not as simple as I thought it was going to be. The main issue
> was that I needed to make sure I didn't break org-store-link. First, I
> made org-link-normalize-string a public function. I then added an
> argument to org-link-normalize-string and
> org-link-heading-search-string. Finally, I replaced
> org-clock--create-link-for-headline, from my previous patch, with
> org-link-create-link-for-table.

Looks reasonable, although I have a couple more comments on the
implementation details.

> Subject: [PATCH] fix: Pipe char (|) in headings breaks clockreport

We will need to add a proper commit message before merging.

> -(defun org-link--normalize-string (string &optional context)
> -  "Remove ignored contents from STRING string and return it.
> +(defun org-link-normalize-string (string &optional context remove-pipes)
> +  "Remove ignored contents from STRING and return it.

It is ok to expose this as public function, but then we should better
create a better API interface.
I feel like adding extra optional argument is going to be a mess in the
long term. We can instead do something like

(cl-defun org-link-normalize-string (string &optional (ignored-contents 
'(statistics-cookie search-syntax)))
"Remove contiguous white spaces and IGNORED-CONTENTS from STRING.
IGNORED-CONTENTS is a list of extra things to remove.  It can be any
subset of (statistics-cookies search-syntax pipe-chars):
- statistics-cookies are statistics cookie strings
- search-syntax is # in #heading and enclosing () in (ref)
- pipe-chars are | characters that may clash with table syntax"

Then, to avoid breakage to the maximum extent, we can also leave the old
org-link--normalize-string as a call to
(org-link-normalize-string string
 (if context
    '(statistics-cookie search-syntax)
   '(search-syntax)))

> +(cl-defun org-link--search-headlines (words &optional remove-pipes)

cl-defun is redundant. A simple defun would do.
I would also call the argument ignore-pipes.

> +  "Search headlines in Org mode buffers.
> +WORDS is a list of strings.  When the value of
> +REMOVE-PIPES is t, pipe chars are removed from
> +headlines before we test for equality.  Ignore
> +COMMENT keyword, TODO keywords, priority cookies,
> +statistics cookies and tags."

Ignore COMMENT keyword, TODO keywords, priority cookies,
statistics cookies, and tags. When REMOVE-PIPES is non-nil,
also ignore pipe characters.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Reply via email to