Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
At Thu, 03 Nov 2011 19:35:01 -0200, Cassio Koshikumo wrote: Hi, all, On the last few days I've been working on a major mode derived from org-mode. While I was at it, I've encoutered some difficulties related to those reported by the OP (Stefan). Specifically: in a lot of places, org-mode will check if it's the current major-mode and, if it's not, will refuse to perform some important task. ... So, finally, here's my question: would it be possible to change all the (eq major-mode 'org-mode) tests and replace them with (derived-mode-p 'org-mode)? Is there any reason not to do this? Please see Carsten's post about the issue: http://article.gmane.org/gmane.emacs.orgmode/47508 If you could provide a list of the functions that need to run in the derived mode or a patch which changes these functions we could put in the `derived-mode-p' selectively. Best, -- David -- OpenPGP... 0x99ADB83B5A4478E6 Jabber dmj...@jabber.org Email. dm...@ictsoc.de pgpIkYAGR4trI.pgp Description: PGP signature
Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
Hi, all, On the last few days I've been working on a major mode derived from org-mode. While I was at it, I've encoutered some difficulties related to those reported by the OP (Stefan). Specifically: in a lot of places, org-mode will check if it's the current major-mode and, if it's not, will refuse to perform some important task. But this poses a problem: a derived mode will always fail the test (eq major-mode 'org-mode) So, as long as org-mode relies on this test to perform some tasks, a derived mode will have non-functional pieces and bugs. Before this patch, all the checking was centralized in the defsubst `org-mode-p'. While I was developing my mode, I just changed it to (defsubst org-mode-p () (derived-mode-p 'org-mode)) Of course, this was just a temporary workaround, but I was actually planning on submitting a patch that implemented the (already discussed) `org-mode-or-derived-mode-p' alternative, replacing all calls to `org-mode-p' with calls to it. But then comes this patch, which removes `org-mode-p' altogether. So, now, there are several instances of the always-failing test on the code. Clearly, this is a big problem for anyone writing derived modes (there doesn't seem to be a lot of people doing that, but still). So, finally, here's my question: would it be possible to change all the (eq major-mode 'org-mode) tests and replace them with (derived-mode-p 'org-mode)? Is there any reason not to do this? That's effectively what happened when I changed the defsubst and I haven't noticed any problems. After all, org-mode will continue to always pass that test -- it's just that the derived modes will start to pass it too. If no ones' against it, I think I could create the patch. I haven't signed the FSF papers, but this change falls neatly into the repetitive category. Cheers, -- Cássio Koshikumo At Sat, 22 Oct 2011 11:31:21 +0200, Bastien wrote: Hi Tassilo, Tassilo Horn tass...@member.fsf.org writes: Ups, I've slightly misread your suggestion. Currently, there is only one place in org-src.el that check for being a mode derived from org-mode or org-mode itself. The second is useless, because (derived-mode-p 'org-mode) is true for org-mode, too. But as I've said, I've misread your suggestion and made a patch that completely erases org-mode-p and all its calls. Feel free to apply or discard it. :-) I applied this patch -- thanks for it. IMHO another reason for using (eq major-mode 'org-mode) instead of (org-mode-p) is that new contributors are likely to use this more explicit (though longer) expression before they find out that a defsubst is available for that. -- Bastien
Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
Hi Tassilo, Tassilo Horn tass...@member.fsf.org writes: Ups, I've slightly misread your suggestion. Currently, there is only one place in org-src.el that check for being a mode derived from org-mode or org-mode itself. The second is useless, because (derived-mode-p 'org-mode) is true for org-mode, too. But as I've said, I've misread your suggestion and made a patch that completely erases org-mode-p and all its calls. Feel free to apply or discard it. :-) I applied this patch -- thanks for it. IMHO another reason for using (eq major-mode 'org-mode) instead of (org-mode-p) is that new contributors are likely to use this more explicit (though longer) expression before they find out that a defsubst is available for that. -- Bastien
Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
Carsten Dominik carsten.domi...@gmail.com writes: Hi Carsten, BTW: I'm not sure if there is any reasonable benefit for `org-mode-p', anyway. Checking the rest of the emacs source tree, then the convention is to either use (eq major-mode 'foo-mode) or (derived-mode-p 'foo-mode) depending on what's needed. I don't see why (org-mode-p) or even (org-mode-p 'derived) is clearer... I agree, it is not clearer, only a bit more compact. I don't think we should have a new function here. Just make a patch that used derived-mode-p in places where this is needed. I would accept such a patch. Ups, I've slightly misread your suggestion. Currently, there is only one place in org-src.el that check for being a mode derived from org-mode or org-mode itself. The second is useless, because (derived-mode-p 'org-mode) is true for org-mode, too. But as I've said, I've misread your suggestion and made a patch that completely erases org-mode-p and all its calls. Feel free to apply or discard it. :-) Bye, Tassilo From 92e83b95aead15a4346e671de6ba211c6ca95a43 Mon Sep 17 00:00:00 2001 From: Tassilo Horn tass...@member.fsf.org Date: Tue, 11 Oct 2011 09:07:27 +0200 Subject: [PATCH] Replace org-mode-p with usual (eq major-mode 'org-mode) check Additionally, replace one (or (org-mode-p) (derived-mode-p 'org-mode)) with (derived-mode-p 'org-mode) cause that is reflexive anyway (returns true, if the current mode is org-mode). Delete one check testing for org-mode or org derived mode --- BUGFIXING/org-log.el |4 +- contrib/lisp/org-annotate-file.el |2 +- contrib/lisp/org-contacts.el |4 +- contrib/lisp/org-expiry.el|4 +- contrib/lisp/org-lparse.el|2 +- contrib/lisp/org-registry.el |2 +- contrib/lisp/org-toc.el |2 +- lisp/org-agenda.el| 26 +++--- lisp/org-archive.el |2 +- lisp/org-ascii.el |2 +- lisp/org-capture.el | 16 lisp/org-colview-xemacs.el|6 ++-- lisp/org-colview.el |6 ++-- lisp/org-ctags.el |2 +- lisp/org-docbook.el |2 +- lisp/org-footnote.el | 14 lisp/org-html.el |2 +- lisp/org-id.el|4 +- lisp/org-indent.el|2 +- lisp/org-latex.el |2 +- lisp/org-macs.el |6 +--- lisp/org-mouse.el |4 +- lisp/org-remember.el |4 +- lisp/org-src.el |8 ++-- lisp/org-table.el |4 +- lisp/org-timer.el |2 +- lisp/org.el | 68 ++-- testing/org-test.el |2 +- 28 files changed, 100 insertions(+), 104 deletions(-) diff --git a/BUGFIXING/org-log.el b/BUGFIXING/org-log.el index 8ebedfe..1fb82e6 100644 --- a/BUGFIXING/org-log.el +++ b/BUGFIXING/org-log.el @@ -21,7 +21,7 @@ (and delete-other-windows (delete-other-windows)) (widen) (goto-char pos) -(when (org-mode-p) +(when (eq major-mode 'org-mode) (org-show-context 'agenda) (save-excursion (and (outline-next-heading) @@ -54,7 +54,7 @@ (switch-to-buffer-other-window buffer) (widen) (goto-char pos) -(when (org-mode-p) +(when (eq major-mode 'org-mode) (org-show-context 'agenda) (save-excursion (and (outline-next-heading) diff --git a/contrib/lisp/org-annotate-file.el b/contrib/lisp/org-annotate-file.el index 265b55f..eb53ab1 100644 --- a/contrib/lisp/org-annotate-file.el +++ b/contrib/lisp/org-annotate-file.el @@ -99,7 +99,7 @@ show the relevant section (concat file: filename :: line) (org-annotate-file-elipsify-desc line (with-current-buffer (find-file org-annotate-file-storage-file) - (unless (org-mode-p) + (unless (eq major-mode 'org-mode) (org-mode)) (goto-char (point-min)) (widen) diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el index f4d9cd7..167caa0 100644 --- a/contrib/lisp/org-contacts.el +++ b/contrib/lisp/org-contacts.el @@ -157,7 +157,7 @@ If both match values are nil, return all contacts. (dolist (file (org-contacts-files)) (org-check-agenda-file file) (with-current-buffer (org-get-agenda-file-buffer file) -(unless (org-mode-p) +(unless (eq major-mode 'org-mode) (error File %s is no in `org-mode' file)) (org-scan-tags '(add-to-list 'markers (set-marker (make-marker) (point))) @@ -263,7 +263,7 @@ If both match values are nil, return all contacts. (when marker (switch-to-buffer-other-window (marker-buffer marker)) (goto-char marker) - (when (org-mode-p) + (when (eq major-mode 'org-mode) (org-show-context 'agenda)
Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
On Sep 5, 2011, at 9:36 AM, Tassilo Horn wrote: Stefan Reichör ste...@xsteve.at writes: Hi Stefan, +(defun org-mode-or-derived-mode-p () + Check if the current buffer is in Org-mode or a derived mode. + (if (derived-mode-p 'org-mode) t nil)) The if is superfluous. And instead of a new function, I'd rather add an optional `derived' parameter to `org-mode-p'. (derived-mode-p 'org-mode) returns either 'org-mode or nil The reason for the if is, that (org-mode-p) returns either t or nil Yes, but from an elisp perspective, 'org-mode is as true as t. BTW: I'm not sure if there is any reasonable benefit for `org-mode-p', anyway. Checking the rest of the emacs source tree, then the convention is to either use (eq major-mode 'foo-mode) or (derived-mode-p 'foo-mode) depending on what's needed. I don't see why (org-mode-p) or even (org-mode-p 'derived) is clearer... I agree, it is not clearer, only a bit more compact. I don't think we should have a new function here. Just make a patch that used derived-mode-p in places where this is needed. I would accept such a patch. - Carsten
Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
Tassilo Horn tass...@member.fsf.org writes: Stefan Reichör ste...@xsteve.at writes: +(defun org-mode-or-derived-mode-p () + Check if the current buffer is in Org-mode or a derived mode. + (if (derived-mode-p 'org-mode) t nil)) The if is superfluous. And instead of a new function, I'd rather add an optional `derived' parameter to `org-mode-p'. (derived-mode-p 'org-mode) returns either 'org-mode or nil The reason for the if is, that (org-mode-p) returns either t or nil The optional derived parameter for org-mode-p is a good idea. Going one step further I think that using a strict parameter would be even better. Because I think that org-mode-p should also return t in derived modes. Only in some rare cases (org-mode-p t) can be used to allow a strict org-mode check. I'd like to preper a patch. Please tell me, if I should use the derived or the strict parameter. Thanks, Stefan.
Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
Stefan Reichör ste...@xsteve.at writes: Hi Stefan, +(defun org-mode-or-derived-mode-p () + Check if the current buffer is in Org-mode or a derived mode. + (if (derived-mode-p 'org-mode) t nil)) The if is superfluous. And instead of a new function, I'd rather add an optional `derived' parameter to `org-mode-p'. (derived-mode-p 'org-mode) returns either 'org-mode or nil The reason for the if is, that (org-mode-p) returns either t or nil Yes, but from an elisp perspective, 'org-mode is as true as t. BTW: I'm not sure if there is any reasonable benefit for `org-mode-p', anyway. Checking the rest of the emacs source tree, then the convention is to either use (eq major-mode 'foo-mode) or (derived-mode-p 'foo-mode) depending on what's needed. I don't see why (org-mode-p) or even (org-mode-p 'derived) is clearer... Bye, Tassilo
[O] [PATCH]: New Add defun org-mode-or-derived-mode-p
2011-09-02 Stefan Reichoer ste...@xsteve.at Add org-mode-or-derived-mode-p to support org-mode derived modes * org-macs.el (org-mode-or-derived-mode-p): New defun * org-src.el (org-edit-src-code): Use org-mode-or-derived-mode-p * org.el (org-entry-properties): Use org-mode-or-derived-mode-p instead of org-mode-p git diff -M HEAD M lisp/org-macs.el M lisp/org-src.el M lisp/org.el diff --git a/lisp/org-macs.el b/lisp/org-macs.el index 13aff02..7603b42 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -276,6 +276,10 @@ we turn off invisibility temporarily. Use this in a `let' form. Check if the current buffer is in Org-mode. (eq major-mode 'org-mode)) +(defun org-mode-or-derived-mode-p () + Check if the current buffer is in Org-mode or a derived mode. + (if (derived-mode-p 'org-mode) t nil)) + (defsubst org-last (list) Return the last element of LIST. (car (last list))) diff --git a/lisp/org-src.el b/lisp/org-src.el index 0e8b1b1..a62f8da 100644 --- a/lisp/org-src.el +++ b/lisp/org-src.el @@ -215,7 +215,7 @@ buffer. (case-fold-search t) (info (org-edit-src-find-region-and-lang)) (full-info (org-babel-get-src-block-info)) - (org-mode-p (or (org-mode-p) (derived-mode-p 'org-mode))) + (org-mode-p (org-mode-or-derived-mode-p)) (beg (make-marker)) (end (make-marker)) (allow-write-back-p (null code)) diff --git a/lisp/org.el b/lisp/org.el index d63b854..d82425c 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -13828,7 +13828,7 @@ things up because then unnecessary parsing is avoided. beg end range props sum-props key key1 value string clocksum) (save-excursion (when (condition-case nil - (and (org-mode-p) (org-back-to-heading t)) + (and (org-mode-or-derived-mode-p) (org-back-to-heading t)) (error nil)) (setq beg (point)) (setq sum-props (get-text-property (point) 'org-summaries))
Re: [O] [PATCH]: New Add defun org-mode-or-derived-mode-p
Stefan Reichör ste...@xsteve.at writes: +(defun org-mode-or-derived-mode-p () + Check if the current buffer is in Org-mode or a derived mode. + (if (derived-mode-p 'org-mode) t nil)) The if is superfluous. And instead of a new function, I'd rather add an optional `derived' parameter to `org-mode-p'. Bye, Tassilo
