Re: [PATCH] Fix a possibly problematic string comparison

2023-08-28 Thread Ihor Radchenko
Rudolf Adamkovič  writes:

> Ihor Radchenko  writes:
>
>> This is not as trivial. Applying this patch will break tests.
>> One needs to carefully examine the org-table logic to fix this
>> particular warning.
>
> Wow!  You are right.  How is that even possible?  In other words, what
> value (of 'elements') can possibly be object-equal to a literal "" but
> not string-equal to the same literal "" or vice versa?

This is not what is happening. 

-   (if (and (eq elements "") (not keep-empty))

(eq elements "") is always nil - the warning is right. We never enter
this branch of `if'.

+   (if (and (string-empty-p elements) (not keep-empty))

After the patch, we do enter this branch of `if', but it breaks tests.

So, it is not just an innocent warning - it reveals a fault in logic
elsewhere in the code.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



[BUG][org-map-entries] Using (widen) in FUNC when SCOPE is equal to 'tree iterates through headlines outside the tree

2023-08-28 Thread Rodrigo Morales
The following result is expected behavior. We iterate through all
headlines starting at headline =* 1=.

#+HEADER: :results value
#+HEADER: :results verbatim
#+begin_src elisp
(with-temp-buffer
  (org-mode)
  (insert
   "* 1
,** 1.1
,** 1.2
,** 1.3
,* 2
,** 2.1
,** 2.2
,** 2.3
")
  (goto-char (point-min))
  (org-map-entries
   (lambda ()
 (org-entry-get nil "ITEM"))
   nil
   'tree))
#+end_src

#+RESULTS:
#+begin_example
("1" "1.1" "1.2" "1.3")
#+end_example

To me, the following result is unexpected behavior, because we are
iterating through headlines that are outside =* 1=, the tree in which
=org-map-entries= was called.

#+begin_quote
SCOPE determines the scope of this command.  It can be any of:

nil The current buffer, respecting the restriction if any
treeThe subtree started with the entry at point
region  The entries within the active region, if any
region-start-level
The entries within the active region, but only those at
the same level than the first one.
#+end_quote

#+HEADER: :results value
#+HEADER: :results verbatim
#+HEADER: :results value
#+begin_src elisp
(with-temp-buffer
  (org-mode)
  (insert
   "* 1
,** 1.1
,** 1.2
,** 1.3
,* 2
,** 2.1
,** 2.2
,** 2.3
")
  (goto-char (point-min))
  (org-map-entries
   (lambda ()
 (widen)
 (org-entry-get nil "ITEM"))
   nil
   'tree))
#+end_src

#+RESULTS:
#+begin_example
("1" "1.1" "1.2" "1.3" "2" "2.1" "2.2" "2.3")
#+end_example

According to the docstring of =org-map-entries=, when SCOPE is =tree=,
we only iterate to the entries under the subtree starting with the
entry at point (relevant part of docstring =org-map-entries= shown
below).

#+begin_quote
SCOPE determines the scope of this command.  It can be any of:

nil The current buffer, respecting the restriction if any
treeThe subtree started with the entry at point
region  The entries within the active region, if any
region-start-level
The entries within the active region, but only those at
the same level than the first one.
#+end_quote

The second result shown above proved this wrong.

* Personal thoughts

I have come up with two possible solutions to handle this scenario.

Solution no. 1: Update the description of =tree= in the docstring of
=org-map-entries= to indicate the user that =widen= shouldn't be used
in FUNC, since otherwise, =org-map-entries= will iterate through
headlines outside the tree at point.

Solution no. 2: Execute the parameter =FUNC=, which is passed to
=org-map-entries=, inside =save-restriction= to ensure that in every
iteration the restriction is not affected, thus only moving forward to
the proper headlines only.



Re: [PATCH] Fix a possibly problematic string comparison

2023-08-28 Thread Rudolf Adamkovič
Ihor Radchenko  writes:

> This is not as trivial. Applying this patch will break tests.
> One needs to carefully examine the org-table logic to fix this
> particular warning.

Wow!  You are right.  How is that even possible?  In other words, what
value (of 'elements') can possibly be object-equal to a literal "" but
not string-equal to the same literal "" or vice versa?

Rudy
-- 
"Logic is a science of the necessary laws of thought, without which no
employment of the understanding and the reason takes place."
-- Immanuel Kant, 1785

Rudolf Adamkovič  [he/him]
Studenohorská 25
84103 Bratislava
Slovakia



Re: What do you recommend for acronyms in org-mode?

2023-08-28 Thread John Kitchin
It is hard to say what the issue is without some working example that shows
performance issues. Usually getting better performance means compromising
some functionality, e.g. not validating citation keys (this requires some
lookup which could take time for large documents/bibtex files), etc.

There are several org-ref-activate-*-links that turn font-lock on and off
for different links.

 org-ref does use some caches, and if these are invalidated frequently,
e.g. by frequently adding new bibtex entries, they have to be rebuilt,
which can also take some time. Some of this is controlled by
bibtex-completion though.

On Sat, Aug 26, 2023 at 9:01 PM Maske  wrote:

> Hi John!
>
> I was using org-ref some time ago, but it seemed to get emacs slower, in
> my low resources PC.
>
> Would you give me some tips to make org-ref lighter?
>
> Best regards
>
> 27 ago 2023 2:33:05 John Kitchin :
>
> You can see how these work in org-ref in this video:
> https://www.youtube.com/watch?v=sebs2vSIEk4=PL0sMmOaE_gs3E0OjExoI7vlCAVygj6S4I=13.
>
>
> On Sat, Aug 26, 2023 at 1:21 PM Maske  wrote:
>
>> I would like to know what do you use for acronyms, glossaries, etc for
>> using inside org-mode? Not for exportation, just to use them inside Emacs.
>>
>> Maybe radio links, or the package org-glossary? org-ref?
>>
>> Best regards
>>
>
>
> --
> John
>
> ---
> Professor John Kitchin (he/his)
> Doherty Hall A207F
> Department of Chemical Engineering
> Carnegie Mellon University
> Pittsburgh, PA 15213
> 412-268-7803
> https://kitchingroup.cheme.cmu.edu
> https://pointbreezepubs.gumroad.com/ pycse bookstore
>
>

-- 
John

---
Professor John Kitchin (he/his)
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
https://kitchingroup.cheme.cmu.edu
https://pointbreezepubs.gumroad.com/ pycse bookstore


Re: [FR] Aligning paragraphs in lists

2023-08-28 Thread Ihor Radchenko
Maske  writes:

> Could be used M-right or M-left to align paragraphs which are 
> immediately below a list?
> ...
> If there are subitems, the text could be needed to be aligned below a 
> superior item, and this must be done in a manual way:
> + Item 1
>    Text indented, belongs to Item 1.
>     - Item 2
>   Text indented, belongs to Item 2.
>    This paragraph is aligned below Item 1, but I think there is too much 
> "friction" to achieve this. And the alignment would be different if the 
> bullet was "1)"

I do not like the idea of changing the behavior of M-/ -
users used to current behavior might be surprised.

But otherwise the feature would be reasonable.
We can make repeated  (indentation) change the current paragraph
indentation in the parent lists. Similar to what we do for new headings:

‘’ (‘org-cycle’)
 In a new entry with no text yet, the first ‘’ demotes the
 entry to become a child of the previous one.  The next ‘’
 makes it a parent, and so on, all the way to top level.  Yet
 another ‘’, and you are back to the initial level.

Patches welcome!

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 




Re: [BUG] When calling org-tree-to-indirect-buffer: (wrong-type-argument listp org-fold-outline) in org-fold-core-get-folding-spec-from-alias [9.6.6 (release_9.6.6 @ /gnu/store/c7vqk20kf6zw73klr8bacnh

2023-08-28 Thread Ihor Radchenko
Adam Porter  writes:

>> This is a very strange backtrace.
>> When I run that `alist-get' call manually, there is no error. And
>> `alist-get' does not call `car'.
>> 
>> May you try to re-generate the backtrace again?
>
> It is indeed strange.  I generated the backtrace several times over 
> several sessions before reporting.  I also can reproduce it in a clean 
> Emacs configuration like so:

You re-define `alist-get' in one of the callers in a way that cannot
handle normal alists like (key . value), not (key . list-value).

(cl-defun burly-follow-url-org-mode ( buffer query)
  "In BUFFER, jump to heading and position from QUERY, and return a buffer.
If QUERY specifies that the buffer should be indirect, a new,
indirect buffer is returned.  Otherwise BUFFER is returned."
  ;; `pcase's map support uses `alist-get', which does not work with string keys
  ;; unless its TESTFN arg is bound to, e.g. `equal', but `map-elt' has 
deprecated
  ;; its TESTFN arg, and there's no way to pass it or bind it when using `pcase'
  ;; anyway.  So we rebind `alist-get' to a function that uses `assoc-string'.
  (with-current-buffer buffer
(cl-letf (((symbol-function 'alist-get)
   (lambda (key alist  _default _remove _testfn)
 ;; Only the first value in the list of values is returned, so 
multiple
 ;; values are not supported.  I don't expect this to be a 
problem...
 (cadr (assoc-string key alist)

Handled.
Not an Org bug.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: Text markup and TeX-style LaTeX fragments spanning more than three lines

2023-08-28 Thread Ihor Radchenko
"Tom Alexander"  writes:

> The documentation for both text markup and TeX-style LaTeX fragments states 
> that their contents (CONTENTS for text markup, BODY for TeX-style LaTeX 
> fragments) cannot span more than three lines, but I'm not seeing such 
> limitation for either in practice. I don't know if this is a documentation 
> error or a parser error.

The parser is the source of truth.
Fixed, in the worg syntax page.
https://git.sr.ht/~bzg/worg/commit/699811bc

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG] ob-python hangs on second start [9.5.4 (release_9.5.4-763-g06373a @ ~/emacs/org-mode/lisp/)]

2023-08-28 Thread Ihor Radchenko
Jack Kamm  writes:

> I think `python-shell-first-prompt-hook' is actually still triggered,
> but rather it is `org-babel-comint-wait-for-output' that is hanging
> while waiting for startup. However, we can just replace it with a
> `sleep-for' instead, because we now wait for
> `org-babel-python--initialized' anyways.
>
> See the attached patch. Please let me know if it fixes the problem for
> you.

LGTM! Feel free to push.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG] Warning when creating preview

2023-08-28 Thread Ihor Radchenko
Edgar Lux  writes:

> On Aug 27, 2023 at 7:29 PM, Ihor Radchenko  wrote:
>> I recommend https://github.com/Malabarba/elisp-bug-hunter to narrow down
>> which part of the config is the culprit.
>
> We have a winner:
>
> ;; Bug hunter error ---
> ;; https://github.com/nicolaisingh/saveplace-pdf-view.git
> (if (file-directory-p "/path/to/packages/saveplace-pdf-view")
> (progn
>   (push "/path/to/packages/saveplace-pdf-view" load-path)
>   (require 'saveplace-pdf-view)
>   (save-place-mode 1)))
> ;; --- end bug hunter error
>
> The error is gone when I comment this out. I know it's not magic, but OMP! 
> (oh, my penguin).

Even with this, I am unable to reproduce.
So, it may only be a part of the problem. Some part of the configuration
before this snippet may be the extra required trigger (in combination
with saveplace-pdf-view).

What happens if you put the save-place-pdf-view snippet near the
beginning of your config and try to bisect again?

> The authors of the package are in Bcc. I like the functionality that it 
> provides and will just ignore the message. If you need more info, let me know.

I do not see any extra BCC.
Also, do note that parser errors might lead to Org file corruption. There
is a reason why we throw this kind of warning into users.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Fix a possibly problematic string comparison

2023-08-28 Thread Ihor Radchenko
Rudolf Adamkovič  writes:

> * lisp/org-table.el (org-table-make-reference): Replace 'eq' with
> 'string-empty-p' to resolve "Warning: 'eq' called with literal string
> that may never match" issued on every 'make' invocation.
>
> - (if (and (eq elements "") (not keep-empty))
> + (if (and (string-empty-p elements) (not keep-empty))

This is not as trivial. Applying this patch will break tests.
One needs to carefully examine the org-table logic to fix this
particular warning.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: Warning(diary.org_archive): Got element without parent (cache active?: t)

2023-08-28 Thread Ihor Radchenko
Gregor Zattler  writes:

> I got these warnings when I followed an id: link
> (org-open-at-point) within diary.org
>
> I have no idea why diary.org_archive was opened at that
> time.  The following is the complete contents of the
> *Warnings* buffer at the moment:

Thanks!
This is again bizarre.
The diagnostics info makes no sense.

Are you seeing the warnings frequently enough to try bisecting your config?

Or, at least, may you also share the value of org-element--cache
variable next time you see the warning.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG] FAILED test-ob-python/session-multiline

2023-08-28 Thread Ihor Radchenko
Jack Kamm  writes:

> This one might take some time to fix, since it's hard to reproduce and
> I'm not sure the cause of it. But here are 3 different solutions I am
> considering now:
>
> 1. In addition to printing `org-babel-python-eoe-indicator' after
>execution, we could also print out a "beginning of execution"
>indicator before execution, and then capture the output between the
>beginning and end indicators. This is how the async session
>execution works, and should avoid any possibility of capturing
>prompts.

This idea looks interesting. Although I would not be so sure that it
will fix things - I have learned that comint has many edge cases we may
not easily anticipate.

For example, see the discussion in
https://yhetil.org/emacs-devel/87y1tgqhmc.fsf@localhost/

> 2. Instead of relying on our own custom `org-babel-python-send-string',
>we could try switching to python.el's
>`python-shell-send-string-no-output', which is probably more
>robust. This would also allow removing the ugly
>`org-babel-python-eoe-indicator' we currently print.

Beware non-standard REPL prompts. When we have something other than >>>,
the logic may have hard time figuring out output boundaries without
deterministic separators like eoe-indicator.

>Downside is that the output would not be echoed into the session
>anymore. To fix that, we could manually insert the captured output
>into the comint session buffer after execution.  Alternatively, we
>could add an argument to `python-shell-send-string-no-output' to
>avoid suppressing output, submit it upstream to python.el, and then
>backport to Org to support older emacs versions.

If we can (eventually) remove some custom code from Org and move it to
Emacs, it will be the best for working towards RMS request
https://orgmode.org/list/e1kiph1-0001lu...@fencepost.gnu.org

> 3. Revisit a series of commits I made in 2020, which was supposed to
>make session evaluation more robust, and was inspired by
>`python-shell-send-string-no-output':
>https://git.sr.ht/~bzg/org-mode/commit/4df12ea39
>
>However, I had to partially revert that work, due to compatibility
>issue with emacs 26.3:
>https://list.orgmode.org/871rjcan53@kyleam.com/
>
>I think we no longer support emacs 26.3, so I could potentially
>revisit this now -- but it's been a few years and will take some
>time to refresh my memory about this.

Note that I changed `org-babel-comint-wait-for-output' and some other
ob-comint machinery since that time. In particular, I tried to address
various edge cases we encountered with bash sessions. See the commentary
in `org-babel-comint-with-output'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG][SECURITY] ob-sqlite header args allows execution of arbitrary shell commands

2023-08-28 Thread Max Nikulin

On 22/08/2023 16:46, Ihor Radchenko wrote:

See the updated version of the patches attached.


Thank you, I do not see apparent issues with code any more. Commit 
message needs an update, apostrophes in the doc string should be 
escaped. Feel free to ignore other comments since there are other issues 
and investing excessive time into polishing of this one is not reasonable.



Subject: [PATCH 1/2] org-macs: New common API function to quote shell
  arguments

* lisp/org-macs.el (org-shell-arg-literal): New auxiliary constant.

  ^
You have changed its name.


(org-make-shell-command): New function that returns shell command
built from individual shell arguments, escaping them to prevent
malicious code execution.

...

+++ b/lisp/org-macs.el
@@ -1593,6 +1593,46 @@ (defun org-sxhash-safe (obj  counter)
  (puthash hash obj org-sxhash-objects)
  (puthash obj hash org-sxhash-hashes)
  
+(defconst org-shell-arg-tag-unescaped (gensym "literal")

+  "Symbol to be used to mark shell arguments that should not be escaped.
+See `org-make-shell-command'.")
+(defun org-make-shell-command (command  args)
+  "Build safe shell command string to run COMMAND with ARGS.
+
+The resulting shell command is safe against malicious shell expansion.
+
+This function is used to avoid unexpected shell expansion when
+building shell command using header arguments from Org babel blocks.
+
+ARGS can be nil, strings, `(,org-shell-arg-tag-unescaped STRING), or a


add \\= before ` and ', otherwise help formatter makes them "pretty".


+list of such elements.  For example,
+
+ (let ((files '(\"a.txt\" \"b.txt\" nil \"$HOME.txt\")))
+  `(org-make-shell-command \"command\" \"-l\"
+  \"value with spaces\"
+  (,org-shell-arg-tag-unescaped \"$HOME\")
+  (mapcar #'identity files)))


Is `mapcar' necessary here? Anyway `delq' is called on another result of 
`mapcar', so the function should not do any destructive list modification.


An idea that may be ignored: make the constant internal and add
(defsubst org-make-shell-command-unescaped (arg)
 (list org--shell-arg-tag-unescaped arg))

to avoid `, noise in `(,org-shell-arg-tag-unescaped STRING).


+will shell-escape \"-l\", \"value with spaces\", and each non-nil member of


There is nothing to escape in "-l".

Perhaps it deserves a mention that COMMAND is passed unquoted to be 
suitable for commands with arguments as defcustom user option values. To 
escape it pass nil as fist argument and add COMMAND before ARGS.



+FILES list, but leave \"$HOME\" to be expanded."


...by shell.


Subject: [PATCH 2/2] org-babel-execute:sqlite: Fix shell arg expansion
  vulnerability

-   (org-fill-template


Should an explicit warning be added to `org-fill-template' that enough 
care is required to escape values if it is used to build a shell command?





Re: [BUG] When calling org-tree-to-indirect-buffer: (wrong-type-argument listp org-fold-outline) in org-fold-core-get-folding-spec-from-alias [9.6.6 (release_9.6.6 @ /gnu/store/c7vqk20kf6zw73klr8bacnh

2023-08-28 Thread Adam Porter

Hi Ihor,

On 8/25/23 03:19, Ihor Radchenko wrote:


Debugger entered--Lisp error: (wrong-type-argument listp org-fold-outline)
   car(org-fold-outline)
   alist-get(org-fold-outline ((:alias . org-link) (org-link . org-link) 
(:alias . org-link-description) (org-link-description . org-link-description) 
(property-drawer . org-fold-drawer) (drawer . org-fold-drawer) (:alias . 
org-fold-drawer) (org-fold-drawer . org-fold-drawer) (verse-block . 
org-fold-block) (src-block . org-fold-block) (special-block . org-fold-block) 
(quote-block . org-fold-block) (export-block . org-fold-block) (example-block . 
org-fold-block) (dynamic-block . org-fold-block) (comment-block . 
org-fold-block) (center-block . org-fold-block) (block . org-fold-block) 
(:alias . org-fold-block) (org-fold-block . org-fold-block) (plain-list . 
org-fold-outline) (inlinetask . org-fold-outline) (outline . org-fold-outline) 
(heading . org-fold-outline) (headline . org-fold-outline) (:alias . 
org-fold-outline) (org-fold-outline . org-fold-outline)))
   org-fold-core-get-folding-spec-from-alias(org-fold-outline)
   org-fold-core--property-symbol-get-create(org-fold-outline)


This is a very strange backtrace.
When I run that `alist-get' call manually, there is no error. And
`alist-get' does not call `car'.

May you try to re-generate the backtrace again?


It is indeed strange.  I generated the backtrace several times over 
several sessions before reporting.  I also can reproduce it in a clean 
Emacs configuration like so:


Using with-emacs.sh on Emacs 29.1:

1.  Run "with-emacs.sh -i burly"
2.  "C-x C-f /tmp/test.org RET"
3.  Input a file like so:

  * Heading A
  ** Heading A1

4.  With point on Heading A1, "C-c C-x b".
5.  "M-x delete-other-windows RET" to show only the subtree buffer.
6.  "M-x burly-bookmark-windows RET", input name, save bookmark.
7.  Kill the file's buffer and delete-other-windows.
8.  "C-x r b" select bookmark that was created, and open it.
9.  You should get the error, and with debug-on-error, the backtrace.

This bug breaks burly's functionality to bookmark and restore subtree 
buffers, which worked fine before upgrading to Emacs 29.1.


Thanks,
Adam