emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] org-capture - using `file' as template
@ 2010-07-14  2:30 Johan Friis
  2010-07-14  7:08 ` David Maus
  2010-07-14  9:04 ` Carsten Dominik
  0 siblings, 2 replies; 6+ messages in thread
From: Johan Friis @ 2010-07-14  2:30 UTC (permalink / raw)
  To: emacs-orgmode


Setting up capture to use templates from file in the latest git seemed
to cause some errors. I looked into it and wrote this small patch. It
could probably use some love, seeing as I am quite new to org-mode,
elisp and emacs in general.

The error I was getting was related to testing (string-match ...) on
a list. The fix is to check if txt is a list before doing the test. In
addition I updated the customize interface to use `file' instead of
`file-contents'. This seems to be the current way of doing things.

Please note: This is my first time posting here. Please let me know if
there are some rules to follow that I missed. And thanks for a great
piece of software :)

Regards,

Johan


diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 2cb6876..da0925c 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -285,7 +285,7 @@ calendar           |  %:type %:date"
 	   (choice :tag "Template"
 		   (string)
 		   (list :tag "File"
-			 (const :format "" file-contents)
+			 (const :format "" file)
 			 (file :tag "Template file"))
 		   (list :tag "Function"
 			 (const :format "" function)
@@ -981,14 +981,15 @@ Point will remain at the first line after the inserted text."
   (org-capture-put :key (car entry) :description (nth 1 entry)
 		   :target (nth 3 entry))
   (let ((txt (nth 4 entry)) (type (or (nth 2 entry) 'entry)))
-    (when (or (not txt) (not (string-match "\\S-" txt)))
-      ;; The template may be empty or omitted for special types.
-      ;; Here we insert the default templates for such cases.
-      (cond
-       ((eq type 'item) (setq txt "- %?"))
-       ((eq type 'checkitem) (setq txt "- [ ] %?"))
-       ((eq type 'table-line) (setq txt "| %? |"))
-       ((member type '(nil entry)) (setq txt "* %?\n  %a"))))
+    (when (or (not txt) (not (listp txt)))
+      (when (or (not txt) (not (string-match "\\S-" txt)))
+        ;; The template may be empty or omitted for special types.
+        ;; Here we insert the default templates for such cases.
+        (cond
+         ((eq type 'item) (setq txt "- %?"))
+         ((eq type 'checkitem) (setq txt "- [ ] %?"))
+         ((eq type 'table-line) (setq txt "| %? |"))
+         ((member type '(nil entry)) (setq txt "* %?\n  %a")))))
     (org-capture-put :template txt :type type)))
 
 (defun org-capture-goto-target (&optional template-key)

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] org-capture - using `file' as template
  2010-07-14  2:30 [PATCH] org-capture - using `file' as template Johan Friis
@ 2010-07-14  7:08 ` David Maus
  2010-07-14 13:54   ` Johan Friis
  2010-07-14  9:04 ` Carsten Dominik
  1 sibling, 1 reply; 6+ messages in thread
From: David Maus @ 2010-07-14  7:08 UTC (permalink / raw)
  To: Johan Friis; +Cc: emacs-orgmode


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

Johan Friis wrote:

>Setting up capture to use templates from file in the latest git seemed
>to cause some errors. I looked into it and wrote this small patch. It
>could probably use some love, seeing as I am quite new to org-mode,
>elisp and emacs in general.

Welcome aboard!

>The error I was getting was related to testing (string-match ...) on
>a list. The fix is to check if txt is a list before doing the test. In
>addition I updated the customize interface to use `file' instead of
>`file-contents'. This seems to be the current way of doing things.

Yep, I can reproduce the error.

Some comments on the proposed fix:

Using (not (listp txt)) works but is not exactly what we want to
check.  To avoid string-match failing txt must be a string.  Obviously
a string is not a list but "not a list" is not necessarily a string.

(when (or (not txt) (stringp txt))
 (when (or (not txt) (not (string-match "\\S-" txt))))
 ...)

If we need to make sure txt is a string before calling string-match,
we can put the check in front of string-match.

(when (or (not txt) (and (stringp txt) (not (string-match "\\S-" txt))))
 ...)

We can do this because the Lisp interpreter leaves the `and' as soon
as one of it's arguments is nil.  I.e. (not (string-match "\\S-" txt))
is not evaluated if (stringp txt) returns nil.

>Please note: This is my first time posting here. Please let me know if
>there are some rules to follow that I missed. And thanks for a great
>piece of software :)

Current rules are documented here:

http://orgmode.org/worg/org-contribute.php

HTH,
  -- David
--
OpenPGP... 0x99ADB83B5A4478E6
Jabber.... dmjena@jabber.org
Email..... dmaus@ictsoc.de

[-- Attachment #1.2: Type: application/pgp-signature, Size: 230 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

_______________________________________________
Emacs-orgmode mailing list
Please use `Reply All' to send replies to the list.
Emacs-orgmode@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-orgmode

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] org-capture - using `file' as template
  2010-07-14  2:30 [PATCH] org-capture - using `file' as template Johan Friis
  2010-07-14  7:08 ` David Maus
@ 2010-07-14  9:04 ` Carsten Dominik
  2010-07-14 18:58   ` Johan Friis
  1 sibling, 1 reply; 6+ messages in thread
From: Carsten Dominik @ 2010-07-14  9:04 UTC (permalink / raw)
  To: Johan Friis, David Maus; +Cc: emacs-orgmode list

Hi Johan, thanks for the report and patch, hi David, thanks for
further comments, I have checked in a fix.

- Carsten

On Jul 14, 2010, at 4:30 AM, Johan Friis wrote:

>
> Setting up capture to use templates from file in the latest git seemed
> to cause some errors. I looked into it and wrote this small patch. It
> could probably use some love, seeing as I am quite new to org-mode,
> elisp and emacs in general.
>
> The error I was getting was related to testing (string-match ...) on
> a list. The fix is to check if txt is a list before doing the test. In
> addition I updated the customize interface to use `file' instead of
> `file-contents'. This seems to be the current way of doing things.
>
> Please note: This is my first time posting here. Please let me know if
> there are some rules to follow that I missed. And thanks for a great
> piece of software :)
>
> Regards,
>
> Johan
>
>
> diff --git a/lisp/org-capture.el b/lisp/org-capture.el
> index 2cb6876..da0925c 100644
> --- a/lisp/org-capture.el
> +++ b/lisp/org-capture.el
> @@ -285,7 +285,7 @@ calendar           |  %:type %:date"
> 	   (choice :tag "Template"
> 		   (string)
> 		   (list :tag "File"
> -			 (const :format "" file-contents)
> +			 (const :format "" file)
> 			 (file :tag "Template file"))
> 		   (list :tag "Function"
> 			 (const :format "" function)
> @@ -981,14 +981,15 @@ Point will remain at the first line after the  
> inserted text."
>   (org-capture-put :key (car entry) :description (nth 1 entry)
> 		   :target (nth 3 entry))
>   (let ((txt (nth 4 entry)) (type (or (nth 2 entry) 'entry)))
> -    (when (or (not txt) (not (string-match "\\S-" txt)))
> -      ;; The template may be empty or omitted for special types.
> -      ;; Here we insert the default templates for such cases.
> -      (cond
> -       ((eq type 'item) (setq txt "- %?"))
> -       ((eq type 'checkitem) (setq txt "- [ ] %?"))
> -       ((eq type 'table-line) (setq txt "| %? |"))
> -       ((member type '(nil entry)) (setq txt "* %?\n  %a"))))
> +    (when (or (not txt) (not (listp txt)))
> +      (when (or (not txt) (not (string-match "\\S-" txt)))
> +        ;; The template may be empty or omitted for special types.
> +        ;; Here we insert the default templates for such cases.
> +        (cond
> +         ((eq type 'item) (setq txt "- %?"))
> +         ((eq type 'checkitem) (setq txt "- [ ] %?"))
> +         ((eq type 'table-line) (setq txt "| %? |"))
> +         ((member type '(nil entry)) (setq txt "* %?\n  %a")))))
>     (org-capture-put :template txt :type type)))
>
> (defun org-capture-goto-target (&optional template-key)
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Please use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

- Carsten

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] org-capture - using `file' as template
  2010-07-14  7:08 ` David Maus
@ 2010-07-14 13:54   ` Johan Friis
  0 siblings, 0 replies; 6+ messages in thread
From: Johan Friis @ 2010-07-14 13:54 UTC (permalink / raw)
  To: emacs-orgmode


Hello David,

Thanks a lot for cleaning up the patch and explaining it. I was not
happy with the double `when' and the double (not txt), but everything
else I tried kept on having bugs. I did not think of checking if txt was
a string. Nice and simple :)

And thanks Carsten for the quick fix. Checking out right now :)

Regards,

Johan

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] org-capture - using `file' as template
  2010-07-14  9:04 ` Carsten Dominik
@ 2010-07-14 18:58   ` Johan Friis
  2010-07-14 20:01     ` Carsten Dominik
  0 siblings, 1 reply; 6+ messages in thread
From: Johan Friis @ 2010-07-14 18:58 UTC (permalink / raw)
  To: emacs-orgmode


Hi Carsten,

I did not have time to try out capture until now, and there is still a
bug. When providing (file "...") as template `org-capture-set-plist'
still generates a default template. Using the patch from David seems to
work fine. Here is a diff against current head.

- Johan

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index f79a20b..0f3a29b 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -981,7 +981,7 @@ Point will remain at the first line after the inserted text."
   (org-capture-put :key (car entry) :description (nth 1 entry)
 		   :target (nth 3 entry))
   (let ((txt (nth 4 entry)) (type (or (nth 2 entry) 'entry)))
-    (when (or (not (stringp txt)) (not (string-match "\\S-" txt)))
+    (when (or (not txt) (and (stringp txt) (not (string-match "\\S-" txt))))
       ;; The template may be empty or omitted for special types.
       ;; Here we insert the default templates for such cases.
       (cond

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: Re: [PATCH] org-capture - using `file' as template
  2010-07-14 18:58   ` Johan Friis
@ 2010-07-14 20:01     ` Carsten Dominik
  0 siblings, 0 replies; 6+ messages in thread
From: Carsten Dominik @ 2010-07-14 20:01 UTC (permalink / raw)
  To: Johan Friis; +Cc: emacs-orgmode

Hi Johan,

thanks for checking after  me - sometimes I should just shut up
and trust others.

Thanks!

- Carsten

On Jul 14, 2010, at 8:58 PM, Johan Friis wrote:

>
> Hi Carsten,
>
> I did not have time to try out capture until now, and there is still a
> bug. When providing (file "...") as template `org-capture-set-plist'
> still generates a default template. Using the patch from David seems  
> to
> work fine. Here is a diff against current head.
>
> - Johan
>
> diff --git a/lisp/org-capture.el b/lisp/org-capture.el
> index f79a20b..0f3a29b 100644
> --- a/lisp/org-capture.el
> +++ b/lisp/org-capture.el
> @@ -981,7 +981,7 @@ Point will remain at the first line after the  
> inserted text."
>   (org-capture-put :key (car entry) :description (nth 1 entry)
> 		   :target (nth 3 entry))
>   (let ((txt (nth 4 entry)) (type (or (nth 2 entry) 'entry)))
> -    (when (or (not (stringp txt)) (not (string-match "\\S-" txt)))
> +    (when (or (not txt) (and (stringp txt) (not (string-match "\ 
> \S-" txt))))
>       ;; The template may be empty or omitted for special types.
>       ;; Here we insert the default templates for such cases.
>       (cond
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Please use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

- Carsten

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-07-14 20:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-14  2:30 [PATCH] org-capture - using `file' as template Johan Friis
2010-07-14  7:08 ` David Maus
2010-07-14 13:54   ` Johan Friis
2010-07-14  9:04 ` Carsten Dominik
2010-07-14 18:58   ` Johan Friis
2010-07-14 20:01     ` Carsten Dominik

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).