[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [ANN] lisp/ob-tangle-sync.el
From: |
Ihor Radchenko |
Subject: |
Re: [ANN] lisp/ob-tangle-sync.el |
Date: |
Wed, 03 May 2023 11:43:51 +0000 |
Mehmet Tekman <mtekman89@gmail.com> writes:
> I've modified the ob-tangle.el file for the main tangling and
> detangling functions. Most importantly, both functions can now
> exchange information from the source Org mode file to the target
> remote tangle file in either direction, depending on whether the
> source Org file has `:tangle-sync <action>' in the header.
Thanks!
> The action is one of:
>
> - "export" = always transmit information from the source Org mode
> block to the target remote file.
> - "import" = always transmit information from the target remote
> file to the source Org mode block.
> - "skip" = skip the block.
> - "both" = transmit information from source block to target block
> or target block to source, depending on whether the
> tangle or detangle is called from the source buffer or
> the target buffer respectively.
May it be better to make :tangle header argument compound?
Like ":tangle "file" export". Similar to :results header argument. See
"16.6 Results of Evaluation" section of Org manual.
Also, some general comments on the patches:
1. Please make sure that patches do not leave Org in broken state in the
middle of the patchset. Your patch #1 adds two `defun's in the middle
of `org-babel-detangle', which is not right.
2. When naming private functions, "--" should be after library prefix:
org-babel--...
3. Please do not use private functions from third-party libraries. I am
talking about `cl--set-buffer-substring' in particular.
4. Please make sure that docstrings clearly describe what the function
does, each of its arguments, and the expected return value.
In the patch, `org-babel-detangle--block-contents', NEAREST is
ambiguous. The actual meaning is "block at point or previous block".
5. Pay attention to buffer boundaries. In particular, remember that
buffer may be narrowed when you call a command and that expressions
like (+ 2 (line-beginning-position)) may resolve beyond
`point-min'/`point-max'.
6. Avoid using cryptic list functions like `cdadar' when you can use
something more readable.
7. When naming functions or macros, make sure that the name is roughly
describing the purpose of the function. In `org-babel-detangle', you
added `single-block-metrics' local function that does not only do the
metrics, but also (unexpectedly!) calls
`org-babel-detangle-single-block'. This is especially important for
local functions that lack docstring.
8. In `org-babel-tangle', (setq block-counter (+ 1 block-counter))
appears to be misplaced into outer sexp level after your patch.
9. In commit messages, please mark newly added functions clearly.
Like "(org-babel-foo): New function. It does this and that."
Same for commit summaries - "lisp/ob-tangle.el: Detangle a single
block" is awkward. You should clearly indicate that you added
something new to the library.
--
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>
- Re: [ANN] lisp/ob-tangle-sync.el, Mehmet Tekman, 2023/05/02
- Re: [ANN] lisp/ob-tangle-sync.el,
Ihor Radchenko <=
- Re: [ANN] lisp/ob-tangle-sync.el, Mehmet Tekman, 2023/05/03
- Re: [ANN] lisp/ob-tangle-sync.el, Mehmet Tekman, 2023/05/03
- Re: [ANN] lisp/ob-tangle-sync.el, Ihor Radchenko, 2023/05/03
- Message not available
- Message not available
- Message not available
- Re: [ANN] lisp/ob-tangle-sync.el, Mehmet Tekman, 2023/05/09
- Re: [ANN] lisp/ob-tangle-sync.el, Ihor Radchenko, 2023/05/10
- Re: [ANN] lisp/ob-tangle-sync.el, mtekman89, 2023/05/10
- Re: [ANN] lisp/ob-tangle-sync.el, Ihor Radchenko, 2023/05/10
- Re: [ANN] lisp/ob-tangle-sync.el, Mehmet Tekman, 2023/05/10
- Re: [ANN] lisp/ob-tangle-sync.el, Ihor Radchenko, 2023/05/12