[PATCH] org-bibtex-yank: Allow to populate existing item

[PATCH] org-bibtex-yank: Allow to populate existing item

[Martin Kampas]

[-- Attachment #1.1: Type: text/plain, Size: 181 bytes --]

Hi,

The attached patch allows to use org-bibtex-yank to 
populate an existing item instead of creating a new one, 
aligning its behavior with org-bibtex-create.

BR,
Martin Kampas

[-- Attachment #1.2: Type: text/html, Size: 635 bytes --]

[-- Attachment #2: 0001-org-bibtex-yank-Allow-to-populate-existing-item.patch --]
[-- Type: text/x-patch, Size: 3834 bytes --]

From 96af3ef46bb056e58206af77d3d37c5af2e43d7f Mon Sep 17 00:00:00 2001
From: Martin Kampas <martin.kampas@ubedi.net>
Date: Mon, 12 Feb 2024 13:24:54 +0100
Subject: [PATCH] org-bibtex-yank: Allow to populate existing item

Align with org-bibtex-create.

* lisp/ol-bibtex.el (org-bibtex-write): New optional argument nonew,
  similar to the existing nonew argument of org-bibtex-create
* lisp/ol-bibtex.el (org-bibtex-yank): New optional argument nonew,
  similar to the existing nonew argument of org-bibtex-create
---
 lisp/ol-bibtex.el | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/lisp/ol-bibtex.el b/lisp/ol-bibtex.el
index c5a950e2d..6ae4ae3cc 100644
--- a/lisp/ol-bibtex.el
+++ b/lisp/ol-bibtex.el
@@ -722,29 +722,31 @@ Return the number of saved entries."
   (interactive "fFile: ")
   (org-bibtex-read-buffer (find-file-noselect file 'nowarn 'rawfile)))
 
-(defun org-bibtex-write (&optional noindent)
+(defun org-bibtex-write (&optional noindent nonew)
   "Insert a heading built from the first element of `org-bibtex-entries'.
 When optional argument NOINDENT is non-nil, do not indent the properties
-drawer."
+drawer. If NONEW is t, add data to the headline of the entry at point."
   (interactive)
   (unless org-bibtex-entries
     (error "No entries in `org-bibtex-entries'"))
   (let* ((entry (pop org-bibtex-entries))
 	 (org-special-properties nil) ; avoids errors with `org-entry-put'
 	 (val (lambda (field) (cdr (assoc field entry))))
-	 (togtag (lambda (tag) (org-toggle-tag tag 'on))))
-    (org-insert-heading)
-    (insert (funcall org-bibtex-headline-format-function entry))
-    (insert "\n:PROPERTIES:\n")
-    (org-bibtex-put "TITLE" (funcall val :title) 'insert)
+	 (togtag (lambda (tag) (org-toggle-tag tag 'on)))
+         (insert-raw (not nonew)))
+    (unless nonew
+      (org-insert-heading)
+      (insert (funcall org-bibtex-headline-format-function entry))
+      (insert "\n:PROPERTIES:\n"))
+    (org-bibtex-put "TITLE" (funcall val :title) insert-raw)
     (org-bibtex-put org-bibtex-type-property-name
 		    (downcase (funcall val :type))
-                    'insert)
+                    insert-raw)
     (dolist (pair entry)
       (pcase (car pair)
 	(:title    nil)
 	(:type     nil)
-	(:key      (org-bibtex-put org-bibtex-key-property (cdr pair) 'insert))
+	(:key      (org-bibtex-put org-bibtex-key-property (cdr pair) insert-raw))
 	(:keywords (if org-bibtex-tags-are-keywords
 		       (dolist (kw (split-string (cdr pair) ", *"))
 			 (funcall
@@ -752,25 +754,28 @@ drawer."
 			  (replace-regexp-in-string
 			   "[^[:alnum:]_@#%]" ""
 			   (replace-regexp-in-string "[ \t]+" "_" kw))))
-		     (org-bibtex-put (car pair) (cdr pair) 'insert)))
-	(_ (org-bibtex-put (car pair) (cdr pair) 'insert))))
-    (insert ":END:\n")
+		     (org-bibtex-put (car pair) (cdr pair) insert-raw)))
+	(_ (org-bibtex-put (car pair) (cdr pair) insert-raw))))
+    (unless nonew
+      (insert ":END:\n"))
     (mapc togtag org-bibtex-tags)
     (unless noindent
       (org-indent-region
        (save-excursion (org-back-to-heading t) (point))
        (point)))))
 
-(defun org-bibtex-yank ()
-  "If kill ring holds a bibtex entry yank it as an Org headline."
-  (interactive)
-  (let (entry)
+(defun org-bibtex-yank (&optional nonew)
+  "If kill ring holds a bibtex entry yank it as an Org headline.
+If nonew is t, add data to the headline of the entry at point."
+  (interactive "P")
+  (let (entry
+        (noindent nonew))
     (with-temp-buffer
       (yank 1)
       (bibtex-mode)
       (setf entry (org-bibtex-read)))
     (if entry
-	(org-bibtex-write)
+	(org-bibtex-write noindent nonew)
       (error "Yanked text does not appear to contain a BibTeX entry"))))
 
 (defun org-bibtex-import-from-file (file)
-- 
2.43.0

Re: [PATCH] org-bibtex-yank: Allow to populate existing item

[Ihor Radchenko]

Martin Kampas <martin.kampas@ubedi.net> writes:

> The attached patch allows to use org-bibtex-yank to 
> populate an existing item instead of creating a new one, 
> aligning its behavior with org-bibtex-create.

Thanks!

> Subject: [PATCH] org-bibtex-yank: Allow to populate existing item
>
> Align with org-bibtex-create.

Please quote like `org-bibtex-create'. Here and in the rest of the
commit message. See https://orgmode.org/worg/org-contribute.html#orgfabdc17

> * lisp/ol-bibtex.el (org-bibtex-write): New optional argument nonew,
>   similar to the existing nonew argument of org-bibtex-create
> * lisp/ol-bibtex.el (org-bibtex-yank): New optional argument nonew,
>   similar to the existing nonew argument of org-bibtex-create

When adding new arguments, we need to announce the change in
etc/ORG_NEWS file. Also, in `org-bibtex-yank', you did not only add an
optional argument, but also modified its behavior with prefix argument.
Changes in prefix arguments should also be announced - they directly
affect all the users.

> -(defun org-bibtex-write (&optional noindent)
> +(defun org-bibtex-write (&optional noindent nonew)
>    "Insert a heading built from the first element of `org-bibtex-entries'.
>  When optional argument NOINDENT is non-nil, do not indent the properties
> -drawer."
> +drawer. If NONEW is t, add data to the headline of the entry at point."

In the code, you do not check for t, but for non-nil. So, please say
"non-nil" in the docstring as well.

Also, I'd prefer a more descriptive name, like update-heading. nonew is
ambiguous - it can be interpreted in several ways.

> -(defun org-bibtex-yank ()
> -  "If kill ring holds a bibtex entry yank it as an Org headline."
> -  (interactive)
> -  (let (entry)
> +(defun org-bibtex-yank (&optional nonew)

> +  "If kill ring holds a bibtex entry yank it as an Org headline.
> +If nonew is t, add data to the headline of the entry at point."

Same here. And please upcase NONEW to indicate that it is a function argument.
Also, document that NONEW is interpreted as interactive prefix argument:

   When called with non-nil prefix argument NONEW, add data to the headline
   of the entry at point.

> +  (interactive "P")
> +  (let (entry
> +        (noindent nonew))

Why do you bind noindent here?

-- 
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: [PATCH] org-bibtex-yank: Allow to populate existing item

[Martin Kampas]

[-- Attachment #1.1: Type: text/plain, Size: 873 bytes --]

On neděle 18. února 2024 15:29:09 CET Ihor Radchenko wrote:
> In the code, you do not check for t, but for non-nil. So, please say
> "non-nil" in the docstring as well.

As stated in the commit message, aligning with org-bibtex-create... ok, let me update the 
doc string of org-bibtex-create as well.

> Also, I'd prefer a more descriptive name, like update-heading. nonew is
> ambiguous - it can be interpreted in several ways.

...aligning with org-bibtex-create.

> > +  (interactive "P")
> > +  (let (entry
> > +        (noindent nonew))
> 
> Why do you bind noindent here?

It says nothing about (re)indenting the existing entry, so my feeling was it would be 
better to avoid doing that if there is no way to override. Now if you need to ask about it, it 
seems it is not really an issue, so I removed it, hardcoding nil there.

BR,
Martin

[-- Attachment #1.2: Type: text/html, Size: 2218 bytes --]

[-- Attachment #2: 0001-org-bibtex-yank-Allow-to-populate-existing-item.patch --]
[-- Type: text/x-patch, Size: 5404 bytes --]

From 8210921aa94430b651c9813acb1c12448db00fb8 Mon Sep 17 00:00:00 2001
From: Martin Kampas <martin.kampas@ubedi.net>
Date: Mon, 12 Feb 2024 13:24:54 +0100
Subject: [PATCH] org-bibtex-yank: Allow to populate existing item

Align with `org-bibtex-create'.

* lisp/ol-bibtex.el (org-bibtex-write): New optional argument `nonew',
  similar to the existing `nonew' argument of `org-bibtex-create'.
* lisp/ol-bibtex.el (org-bibtex-yank): New optional argument `nonew',
  similar to the existing `nonew' argument of `org-bibtex-create'.
---
 etc/ORG-NEWS      | 10 ++++++++++
 lisp/ol-bibtex.el | 42 ++++++++++++++++++++++++------------------
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 7d2f70ab6..4751deaaa 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -812,6 +812,11 @@ propagating the value for this variable to ~org-agenda-archives-mode~.
 For acceptable values and their meaning, see the value of that variable.
 
 ** New features
+*** ~org-bibtex-yank~ accepts a prefix argument
+
+When called with a prefix argument, ~org-bibtex-yank~ adds data to the
+headline of the entry at point instead of creating a new one.
+
 *** =ob-plantuml.el=: Support tikz file format output
 
 =ob-plantuml.el= now output =tikz= :file format via
@@ -1011,6 +1016,11 @@ The same can be done via startup options:
 : #+STARTUP: fnanon
 
 ** New functions and changes in function arguments
+*** New optional argument =NONEW= for ~org-bibtex-yank~
+
+When the new argument is non-nil, add data to the headline of the
+entry at point.
+
 *** New API functions to store data within ~org-element-cache~
 
 Elisp programs can now store data inside Org element cache.
diff --git a/lisp/ol-bibtex.el b/lisp/ol-bibtex.el
index c5a950e2d..4ec4dc958 100644
--- a/lisp/ol-bibtex.el
+++ b/lisp/ol-bibtex.el
@@ -642,8 +642,8 @@ With prefix argument OPTIONAL also prompt for optional fields."
 
 (defun org-bibtex-create (&optional arg nonew)
   "Create a new entry at the given level.
-With a prefix arg, query for optional fields as well.
-If nonew is t, add data to the headline of the entry at point."
+With a prefix ARG, query for optional fields as well.
+If NONEW is non-nil, add data to the headline of the entry at point."
   (interactive "P")
   (let* ((type (completing-read
 		"Type: " (mapcar (lambda (type)
@@ -722,29 +722,32 @@ Return the number of saved entries."
   (interactive "fFile: ")
   (org-bibtex-read-buffer (find-file-noselect file 'nowarn 'rawfile)))
 
-(defun org-bibtex-write (&optional noindent)
+(defun org-bibtex-write (&optional noindent nonew)
   "Insert a heading built from the first element of `org-bibtex-entries'.
 When optional argument NOINDENT is non-nil, do not indent the properties
-drawer."
+drawer.  If NONEW is non-nil, add data to the headline of the entry at
+point."
   (interactive)
   (unless org-bibtex-entries
     (error "No entries in `org-bibtex-entries'"))
   (let* ((entry (pop org-bibtex-entries))
 	 (org-special-properties nil) ; avoids errors with `org-entry-put'
 	 (val (lambda (field) (cdr (assoc field entry))))
-	 (togtag (lambda (tag) (org-toggle-tag tag 'on))))
-    (org-insert-heading)
-    (insert (funcall org-bibtex-headline-format-function entry))
-    (insert "\n:PROPERTIES:\n")
-    (org-bibtex-put "TITLE" (funcall val :title) 'insert)
+	 (togtag (lambda (tag) (org-toggle-tag tag 'on)))
+         (insert-raw (not nonew)))
+    (unless nonew
+      (org-insert-heading)
+      (insert (funcall org-bibtex-headline-format-function entry))
+      (insert "\n:PROPERTIES:\n"))
+    (org-bibtex-put "TITLE" (funcall val :title) insert-raw)
     (org-bibtex-put org-bibtex-type-property-name
 		    (downcase (funcall val :type))
-                    'insert)
+                    insert-raw)
     (dolist (pair entry)
       (pcase (car pair)
 	(:title    nil)
 	(:type     nil)
-	(:key      (org-bibtex-put org-bibtex-key-property (cdr pair) 'insert))
+	(:key      (org-bibtex-put org-bibtex-key-property (cdr pair) insert-raw))
 	(:keywords (if org-bibtex-tags-are-keywords
 		       (dolist (kw (split-string (cdr pair) ", *"))
 			 (funcall
@@ -752,25 +755,28 @@ drawer."
 			  (replace-regexp-in-string
 			   "[^[:alnum:]_@#%]" ""
 			   (replace-regexp-in-string "[ \t]+" "_" kw))))
-		     (org-bibtex-put (car pair) (cdr pair) 'insert)))
-	(_ (org-bibtex-put (car pair) (cdr pair) 'insert))))
-    (insert ":END:\n")
+		     (org-bibtex-put (car pair) (cdr pair) insert-raw)))
+	(_ (org-bibtex-put (car pair) (cdr pair) insert-raw))))
+    (unless nonew
+      (insert ":END:\n"))
     (mapc togtag org-bibtex-tags)
     (unless noindent
       (org-indent-region
        (save-excursion (org-back-to-heading t) (point))
        (point)))))
 
-(defun org-bibtex-yank ()
-  "If kill ring holds a bibtex entry yank it as an Org headline."
-  (interactive)
+(defun org-bibtex-yank (&optional nonew)
+  "If kill ring holds a bibtex entry yank it as an Org headline.
+When called with non-nil prefix argument NONEW, add data to the
+headline of the entry at point."
+  (interactive "P")
   (let (entry)
     (with-temp-buffer
       (yank 1)
       (bibtex-mode)
       (setf entry (org-bibtex-read)))
     (if entry
-	(org-bibtex-write)
+	(org-bibtex-write nil nonew)
       (error "Yanked text does not appear to contain a BibTeX entry"))))
 
 (defun org-bibtex-import-from-file (file)
-- 
2.43.0

Re: [PATCH] org-bibtex-yank: Allow to populate existing item

[Ihor Radchenko]

Martin Kampas <martin.kampas@ubedi.net> writes:

> Subject: [PATCH] org-bibtex-yank: Allow to populate existing item

Applied, onto main, with amendments.
I changed the argument name from NONEW to UPDATE-HEADING and updated the
commit message.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=37e468cf1

Thanks for your contribution!

-- 
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: [PATCH] org-bibtex-yank: Allow to populate existing item

[Martin Kampas]

[-- Attachment #1.1: Type: text/plain, Size: 406 bytes --]

Ihor Radchenko wrote:
> Martin Kampas <martin.kampas@ubedi.net> writes:
> > Subject: [PATCH] org-bibtex-yank: Allow to populate existing item
> 
> Applied, onto main, with amendments.
> I changed the argument name from NONEW to UPDATE-HEADING and 
updated the
> commit message.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?
id=37e468cf1
> 
> Thanks for your contribution!

Thank you too!

[-- Attachment #1.2: Type: text/html, Size: 1370 bytes --]

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]