emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: Mehmet Tekman <mtekman89@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [ANN] lisp/ob-tangle-sync.el
Date: Thu, 03 Aug 2023 07:35:22 +0000	[thread overview]
Message-ID: <87wmyc1sud.fsf@localhost> (raw)
In-Reply-To: <CAHHeYz+_HGfZR=A3-P16+yt+D0JR0T1NO67Urvo0XRGKiFmEmA@mail.gmail.com>

Mehmet Tekman <mtekman89@gmail.com> writes:

>    It's a big patch mostly, because there were no intermediate commits
>    in which the org framework wouldn't be broken. Hope that's okay!

Sure it is.

> ** Problems
>
> It seems to work well for most tests, except for the "file with
> spaces.txt" which I'm not sure how to proceed with. I feel like
> patching at the level `org-babel--merge-params' is the wrong way to
> go, and that I should patch it further upstream.
>
> That upstream leads to `org-babel-read' which currently chomps the
> quotes off anything it gets:
>
> #+begin_src elisp
>   (org-babel-read " \"my file with quotes\" ")
> #+end_src

> I don't know if this is the expected behaviour for quoted strings with
> spaces, so before I proceed trying to patch this I thought I would
> check in with you and ask whether I should start patching here or not.

This is indeed a problem.
Org cannot distinguish between

#+begin_src lang :foo value with spaces
and
#+begin_src lang :foo "value with spaces"

What we can do it make `org-babel-read' assign a text property to the
resulting string when it is a string with quotes:

(org-babel-read "my file with quotes") ; => "my file with quotes"
(org-babel-read "\"my file with quotes\"") ; => #("my file with quotes" 0 19 (org-babel-quote t))

Then, we can later use this information in `org-babel-merge-params'.
We will not call `split-string', when 'org-babel-quote text property is
present.
Also, `split-string' won't work when we have something like

"yes \"my file with quotes\""

Instead, we should use
(mapcar #'org-babel-read (org-babel-balanced-split "yes \"my file with quotes\"" ?\s))

> -    (tangle	. ((tangle yes no :any)))
> +    (tangle	. ((tangle yes no :any)
> +                  (import export skip sync)))

We should not yet change this before the code using the new tangle
values is in place.

> +(defun org-babel--merge-headers (exclusive-groups &rest result-params)
> +  "Maintain exclusivity of mutually exclusive parameters, as defined
> +in EXCLUSIVE-GROUPS while merging lists in RESULT-PARAMS."

Please update the docstring, following conventions:
1. First line should be a short full sentence explaining what function
does briefly.
2. Each argument should be explained - what it is and what is the
   structure.

Also, RESULT-PARAMS is confusing because it is no longer just used for
:results. Please rename.

> +  (cl-flet ((alist-append (alist key val)
> +              (let ((existing (cdr (assoc key alist))))
> +                (if existing
> +                    (setcdr (assoc key alist) (append existing (list val)))
> +                  (setq alist (cons (cons key (list val)) alist)))
> +                alist)))

Can simply
(push val (alist-get key alist nil nil #'equal))

> +    ;; Problem: Having an :any keyword in an exclusion group means
> +    ;;          that a parameter of "yes" could match to an exclusion
> +    ;;          group that contains both "yes" AND ":any".
> +    ;; Solution: For each parameter, build a list of exclusion groups
> +    ;;           it could belong to. If a parameter belongs to two
> +    ;;           groups, eliminate it from the group that contains the
> +    ;;           ":any" keyword.
> +    ;;
> +    ;;  Details: Exclusion groups (e.g.'("tangle" "yes" "no" ":any") )
> +    ;;           are referenced to by their car ("tangle"), acting as
> +    ;;           a key for the entire group.
> +    ;;

This is not a "problem" - just something to consider. You can omit
"Problem:", "Solution:", and "Details:" converting into ordinary
paragraphs.

> +    ;; Assumption: The order of RESULT-PARAMS dictate the hierarchy of
> +    ;;             the cascading headers.

The expected function parameters should be described in the docstring,
not in a comment.

> +    (let ((any-group-key       ;; exclusion group key for group with :any keyword
> +           (caar (cl-remove-if-not (lambda (x) (member ":any" x)) exclusive-groups)))

`cl-find' will be simpler.

> +      (delete-dups unexplained-params)

Should be
(setq unexplained-params (delete-dups unexplained-params))

`delete-dups' _returns_ the modified list, which may or may not preserve
the initial pointer to the first cons cell.

> +        ;; Set values in the same order that the exclusion groups are defined
> +        (let ((group-key-order (mapcar #'car exclusive-groups)))
> +          (setq group-alist (cl-remove-if-not #'identity

Or just delq nil

> +                                              (mapcar (lambda (x) (car (alist-get x group-alist)))
> +                                                      group-key-order))))

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>


  parent reply	other threads:[~2023-08-03  7:36 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-26 14:48 [ANN] lisp/ob-tangle-sync.el Mehmet Tekman
2023-04-26 16:43 ` John Wiegley
2023-04-26 18:43   ` Mehmet Tekman
2023-04-27  2:55 ` Ruijie Yu via General discussions about Org-mode.
2023-04-27  6:27   ` Mehmet Tekman
2023-04-28 10:57     ` Ruijie Yu via General discussions about Org-mode.
2023-04-28 11:28       ` Mehmet Tekman
2023-05-02 20:43         ` Mehmet Tekman
2023-05-03  2:31           ` Ruijie Yu via General discussions about Org-mode.
2023-05-03  7:53             ` Mehmet Tekman
2023-05-03  8:34               ` Mehmet Tekman
2023-05-03  8:44                 ` Ihor Radchenko
2023-05-03 11:43           ` Ihor Radchenko
2023-05-03 13:54             ` Mehmet Tekman
2023-05-03 18:06               ` Ihor Radchenko
2023-05-03 15:05             ` Mehmet Tekman
2023-05-03 15:21               ` Ihor Radchenko
     [not found]                 ` <87lei577g4.fsf@gmail.com>
     [not found]                   ` <87lei5v1fg.fsf@localhost>
     [not found]                     ` <87fs8duyae.fsf@localhost>
2023-05-09 14:03                       ` Mehmet Tekman
2023-05-10  9:46                         ` Ihor Radchenko
2023-05-10 11:06                           ` mtekman89
2023-05-10 11:32                             ` Ihor Radchenko
2023-05-10 16:20                               ` Mehmet Tekman
2023-05-12 12:33                                 ` Ihor Radchenko
2023-05-16 12:49                                   ` Mehmet Tekman
2023-05-16 18:57                                     ` Ihor Radchenko
2023-05-17 13:45                                       ` Mehmet Tekman
2023-05-18 10:30                                         ` Ihor Radchenko
2023-05-19  7:10                                           ` Mehmet Tekman
2023-07-15 12:38                                             ` Ihor Radchenko
2023-07-16  9:42                                               ` Mehmet Tekman
2023-07-17 11:29                                                 ` Mehmet Tekman
2023-07-18  8:47                                                   ` Ihor Radchenko
2023-07-21  8:48                                                     ` Mehmet Tekman
2023-07-22  8:02                                                       ` Ihor Radchenko
2023-07-25 11:19                                                         ` Mehmet Tekman
2023-07-25 16:19                                                           ` Ihor Radchenko
2023-07-31 13:41                                                             ` Mehmet Tekman
2023-07-31 16:38                                                               ` Ihor Radchenko
2023-07-31 20:11                                                                 ` Mehmet Tekman
2023-08-01  7:54                                                                   ` Ihor Radchenko
2023-08-01  8:49                                                                     ` Mehmet Tekman
2023-08-01  9:30                                                                       ` Ihor Radchenko
2023-08-01 18:19                                                                         ` Bastien Guerry
2023-08-02  7:29                                                                           ` Ihor Radchenko
2023-08-02 14:46                                                                   ` Mehmet Tekman
2023-08-03  6:32                                                                     ` Mehmet Tekman
2023-08-03  7:35                                                                     ` Ihor Radchenko [this message]
2023-08-03  8:08                                                                       ` Mehmet Tekman
2023-08-03  8:16                                                                         ` Ihor Radchenko
     [not found]                                                                           ` <CAHHeYzL6Z5_gGbTUrNzKDh5swgCSQiYsSj3Cs0gFy_d=eXbSBA@mail.gmail.com>
     [not found]                                                                             ` <87o7jo1q2s.fsf@localhost>
2023-08-03  8:46                                                                               ` Mehmet Tekman
2023-08-04  7:41                                                                                 ` Mehmet Tekman
2023-08-04  8:09                                                                                   ` Ihor Radchenko
2023-08-04 13:14                                                                                     ` Mehmet Tekman
2023-08-04 16:34                                                                                       ` Mehmet Tekman
2023-08-06  9:07                                                                                         ` Ihor Radchenko
2023-08-08 19:41                                                                                           ` Mehmet Tekman
2023-08-08 19:51                                                                                             ` Ihor Radchenko
2023-08-08 20:04                                                                                               ` Mehmet Tekman
2023-08-09  8:04                                                                                                 ` Ihor Radchenko
2023-08-05  8:48                                                                                       ` Ihor Radchenko
2023-08-05 22:54                                                                                         ` Mehmet Tekman
2023-11-10  9:41                                                                                           ` Ihor Radchenko
2023-11-10  9:53                                                                                             ` Mehmet Tekman
2023-12-11 13:40                                                                                               ` Ihor Radchenko
2023-12-11 14:28                                                                                                 ` Mehmet Tekman
2023-04-27 12:02 ` Ihor Radchenko
2023-04-27 13:01   ` Mehmet Tekman

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=87wmyc1sud.fsf@localhost \
    --to=yantar92@posteo.net \
    --cc=emacs-orgmode@gnu.org \
    --cc=mtekman89@gmail.com \
    /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).