emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Typo in 'org-without-partial-completion'
@ 2011-06-22 21:08 Paul Sexton
  2011-06-28 16:48 ` Bastien
  2011-06-30  9:12 ` Bastien
  0 siblings, 2 replies; 10+ messages in thread
From: Paul Sexton @ 2011-06-22 21:08 UTC (permalink / raw)
  To: emacs-orgmode

I think there's an error in 'org-without-partial-completion' in org-macs.el.
The variable pc-mode gets bound to the value of partial-completion-mode - but 
this is a VARIABLE (t if that mode is enabled). Funcalling the value of 
the variable produces an error, unsurprisingly. This breaks insertion of 
properties with 'org-set-property'. 

Fixing it involves quoting the the symbol as shown below:


(defmacro org-without-partial-completion (&rest body)
   `(let ((pc-mode (and (boundp 'partial-completion-mode)
                        'partial-completion-mode)))   ; <-- quote added
      (unwind-protect
          (progn
            (when pc-mode (funcall pc-mode -1))
            ,@body)
        (when pc-mode (funcall pc-mode 1)))))

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-06-22 21:08 Typo in 'org-without-partial-completion' Paul Sexton
@ 2011-06-28 16:48 ` Bastien
  2011-06-30  9:12 ` Bastien
  1 sibling, 0 replies; 10+ messages in thread
From: Bastien @ 2011-06-28 16:48 UTC (permalink / raw)
  To: Paul Sexton; +Cc: emacs-orgmode

Hi Paul,

Paul Sexton <psexton@xnet.co.nz> writes:

> I think there's an error in 'org-without-partial-completion' in org-macs.el.
> The variable pc-mode gets bound to the value of partial-completion-mode - but 
> this is a VARIABLE (t if that mode is enabled). Funcalling the value of 
> the variable produces an error, unsurprisingly. This breaks insertion of 
> properties with 'org-set-property'. 
>
> Fixing it involves quoting the the symbol as shown below:
>
>
> (defmacro org-without-partial-completion (&rest body)
>    `(let ((pc-mode (and (boundp 'partial-completion-mode)
>                         'partial-completion-mode)))   ; <-- quote added
>       (unwind-protect
>           (progn
>             (when pc-mode (funcall pc-mode -1))
>             ,@body)
>         (when pc-mode (funcall pc-mode 1)))))

You're right -- thanks for spotting this, and for the clear analysis.

I've committed a patch.

-- 
 Bastien

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-06-22 21:08 Typo in 'org-without-partial-completion' Paul Sexton
  2011-06-28 16:48 ` Bastien
@ 2011-06-30  9:12 ` Bastien
  2011-06-30 19:28   ` David Maus
  1 sibling, 1 reply; 10+ messages in thread
From: Bastien @ 2011-06-30  9:12 UTC (permalink / raw)
  To: Paul Sexton; +Cc: emacs-orgmode

Hi Paul,

Paul Sexton <psexton@xnet.co.nz> writes:

> I think there's an error in 'org-without-partial-completion' in org-macs.el.
> The variable pc-mode gets bound to the value of partial-completion-mode - but 
> this is a VARIABLE (t if that mode is enabled). Funcalling the value of 
> the variable produces an error, unsurprisingly. This breaks insertion of 
> properties with 'org-set-property'. 
>
> Fixing it involves quoting the the symbol as shown below:
>
>
> (defmacro org-without-partial-completion (&rest body)
>    `(let ((pc-mode (and (boundp 'partial-completion-mode)
>                         'partial-completion-mode)))   ; <-- quote added
>       (unwind-protect
>           (progn
>             (when pc-mode (funcall pc-mode -1))
>             ,@body)
>         (when pc-mode (funcall pc-mode 1)))))

I've just reverted this modification, per Sebastian report.

Can you be more precise about the problem it creates with
org-set-property?

Can you check if this version fixes the problems, if any?

#+begin_src emacs-lisp
(defmacro org-without-partial-completion (&rest body)
  `(let ((pc-mode ,(and (boundp 'partial-completion-mode)
			'partial-completion-mode)))
     (unwind-protect
	 (progn
	   (when pc-mode (funcall pc-mode -1))
	   ,@body)
       (when pc-mode (funcall pc-mode 1)))))
#+end_src emacs-lisp

Thanks!

-- 
 Bastien

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-06-30  9:12 ` Bastien
@ 2011-06-30 19:28   ` David Maus
  2011-07-02  7:34     ` David Maus
  0 siblings, 1 reply; 10+ messages in thread
From: David Maus @ 2011-06-30 19:28 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Paul Sexton

[-- Attachment #1: Type: text/plain, Size: 3188 bytes --]

At Thu, 30 Jun 2011 11:12:15 +0200,
Bastien wrote:
> 
> Hi Paul,
> 
> Paul Sexton <psexton@xnet.co.nz> writes:
> 
> > I think there's an error in 'org-without-partial-completion' in org-macs.el.
> > The variable pc-mode gets bound to the value of partial-completion-mode - but 
> > this is a VARIABLE (t if that mode is enabled). Funcalling the value of 
> > the variable produces an error, unsurprisingly. This breaks insertion of 
> > properties with 'org-set-property'. 
> >
> > Fixing it involves quoting the the symbol as shown below:
> >
> >
> > (defmacro org-without-partial-completion (&rest body)
> >    `(let ((pc-mode (and (boundp 'partial-completion-mode)
> >                         'partial-completion-mode)))   ; <-- quote added
> >       (unwind-protect
> >           (progn
> >             (when pc-mode (funcall pc-mode -1))
> >             ,@body)
> >         (when pc-mode (funcall pc-mode 1)))))
> 
> I've just reverted this modification, per Sebastian report.
> 
> Can you be more precise about the problem it creates with
> org-set-property?
> 
> Can you check if this version fixes the problems, if any?
> 
> #+begin_src emacs-lisp
> (defmacro org-without-partial-completion (&rest body)
>   `(let ((pc-mode ,(and (boundp 'partial-completion-mode)
> 			'partial-completion-mode)))
>      (unwind-protect
> 	 (progn
> 	   (when pc-mode (funcall pc-mode -1))
> 	   ,@body)
>        (when pc-mode (funcall pc-mode 1)))))
> #+end_src emacs-lisp

No, I think this won't work. On compile time the byte compiler will
expand the macro and place the expansion in the byte compiled
lisp. Thus it will evaluate the ,(and ...) condition at compile time.

http://www.gnu.org/software/emacs/elisp/html_node/Compiling-Macros.html#Compiling-Macros

#+begin_quote
When a macro call appears in a Lisp program being compiled, the Lisp
compiler calls the macro definition just as the interpreter would, and
receives an expansion. But instead of evaluating this expansion, it
compiles the expansion as if it had appeared directly in the
program. As a result, the compiled code produces the value and side
effects intended for the macro, but executes at full compiled
speed. This would not work if the macro body computed the value and
side effects itself—they would be computed at compile time, which is
not useful.
#+end_quote

What about this:

#+begin_src emacs-lisp
  (defmacro org-without-partial-completion (&rest body)
    `(let ((pc-mode-p (and (boundp 'partial-completion-mode)
                           (fboundp 'partial-completion-mode))))
       (when pc-mode-p
         (unwind-protect
             (progn
               (partial-completion-mode -1)
               ,@body)
           (partial-completion-mode 1)))))
#+end_src

This will turn off partial-completion-mode if the symbol is non-nil
and callable.

For Sebastien's problem: Strange thing. Looks like the symbol
partial-completion-mode is non-nil but not callable. 

Maybe an Emacs 24 development version issue?

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-06-30 19:28   ` David Maus
@ 2011-07-02  7:34     ` David Maus
  2011-07-02  9:26       ` Bastien
  2011-07-02 14:40       ` Nick Dokos
  0 siblings, 2 replies; 10+ messages in thread
From: David Maus @ 2011-07-02  7:34 UTC (permalink / raw)
  To: David Maus; +Cc: Bastien, Paul Sexton, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 3695 bytes --]

At Thu, 30 Jun 2011 21:28:08 +0200,
David Maus wrote:
> 
> At Thu, 30 Jun 2011 11:12:15 +0200,
> Bastien wrote:
> > 
> > Hi Paul,
> > 
> > Paul Sexton <psexton@xnet.co.nz> writes:
> > 
> > > I think there's an error in 'org-without-partial-completion' in org-macs.el.
> > > The variable pc-mode gets bound to the value of partial-completion-mode - but 
> > > this is a VARIABLE (t if that mode is enabled). Funcalling the value of 
> > > the variable produces an error, unsurprisingly. This breaks insertion of 
> > > properties with 'org-set-property'. 
> > >
> > > Fixing it involves quoting the the symbol as shown below:
> > >
> > >
> > > (defmacro org-without-partial-completion (&rest body)
> > >    `(let ((pc-mode (and (boundp 'partial-completion-mode)
> > >                         'partial-completion-mode)))   ; <-- quote added
> > >       (unwind-protect
> > >           (progn
> > >             (when pc-mode (funcall pc-mode -1))
> > >             ,@body)
> > >         (when pc-mode (funcall pc-mode 1)))))
> > 
> > I've just reverted this modification, per Sebastian report.
> > 
> > Can you be more precise about the problem it creates with
> > org-set-property?
> > 
> > Can you check if this version fixes the problems, if any?
> > 
> > #+begin_src emacs-lisp
> > (defmacro org-without-partial-completion (&rest body)
> >   `(let ((pc-mode ,(and (boundp 'partial-completion-mode)
> > 			'partial-completion-mode)))
> >      (unwind-protect
> > 	 (progn
> > 	   (when pc-mode (funcall pc-mode -1))
> > 	   ,@body)
> >        (when pc-mode (funcall pc-mode 1)))))
> > #+end_src emacs-lisp
> 
> No, I think this won't work. On compile time the byte compiler will
> expand the macro and place the expansion in the byte compiled
> lisp. Thus it will evaluate the ,(and ...) condition at compile time.
> 
> http://www.gnu.org/software/emacs/elisp/html_node/Compiling-Macros.html#Compiling-Macros
> 
> #+begin_quote
> When a macro call appears in a Lisp program being compiled, the Lisp
> compiler calls the macro definition just as the interpreter would, and
> receives an expansion. But instead of evaluating this expansion, it
> compiles the expansion as if it had appeared directly in the
> program. As a result, the compiled code produces the value and side
> effects intended for the macro, but executes at full compiled
> speed. This would not work if the macro body computed the value and
> side effects itself—they would be computed at compile time, which is
> not useful.
> #+end_quote
> 
> What about this:
> 
> #+begin_src emacs-lisp
>   (defmacro org-without-partial-completion (&rest body)
>     `(let ((pc-mode-p (and (boundp 'partial-completion-mode)
>                            (fboundp 'partial-completion-mode))))
>        (when pc-mode-p
>          (unwind-protect
>              (progn
>                (partial-completion-mode -1)
>                ,@body)
>            (partial-completion-mode 1)))))
> #+end_src
> 
> This will turn off partial-completion-mode if the symbol is non-nil
> and callable.


Or even better:

#+begin_src emacs-lisp
  (defmacro org-without-partial-completion (&rest body)
    `(when (and (boundp 'partial-completion-mode)
                (fboundp 'partial-completion-mode))
       (unwind-protect
           (progn
             (partial-completion-mode -1)
             ,@body)
         (partial-completion-mode 1))))
#+end_src

This avoids leaking if 'body happens to uses a symbol 'pc-mode-p in a
different context.

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-07-02  7:34     ` David Maus
@ 2011-07-02  9:26       ` Bastien
  2011-07-02 13:50         ` David Maus
  2011-07-02 14:40       ` Nick Dokos
  1 sibling, 1 reply; 10+ messages in thread
From: Bastien @ 2011-07-02  9:26 UTC (permalink / raw)
  To: David Maus; +Cc: Paul Sexton, emacs-orgmode

Hi David and Paul,

David Maus <dmaus@ictsoc.de> writes:

> #+begin_src emacs-lisp
>   (defmacro org-without-partial-completion (&rest body)
>     `(when (and (boundp 'partial-completion-mode)
>                 (fboundp 'partial-completion-mode))
>        (unwind-protect
>            (progn
>              (partial-completion-mode -1)
>              ,@body)
>          (partial-completion-mode 1))))
> #+end_src

That's clearly better -- thanks for this.  I applied this change, 
thanks for this improvement!

Paul, please let me know if things don't behave as expected.

-- 
 Bastien

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-07-02  9:26       ` Bastien
@ 2011-07-02 13:50         ` David Maus
  0 siblings, 0 replies; 10+ messages in thread
From: David Maus @ 2011-07-02 13:50 UTC (permalink / raw)
  To: Bastien; +Cc: David Maus, Paul Sexton, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

At Sat, 02 Jul 2011 11:26:59 +0200,
Bastien wrote:
>
> Hi David and Paul,
>
> David Maus <dmaus@ictsoc.de> writes:
>
> > #+begin_src emacs-lisp
> >   (defmacro org-without-partial-completion (&rest body)
> >     `(when (and (boundp 'partial-completion-mode)
> >                 (fboundp 'partial-completion-mode))
> >        (unwind-protect
> >            (progn
> >              (partial-completion-mode -1)
> >              ,@body)
> >          (partial-completion-mode 1))))
> > #+end_src
>
> That's clearly better -- thanks for this.  I applied this change,
> thanks for this improvement!
>
> Paul, please let me know if things don't behave as expected.

da086f9606d1875b5270dd367d7f5326b7ed8e9a ;)

david > (sloppy-mode -1)

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-07-02  7:34     ` David Maus
  2011-07-02  9:26       ` Bastien
@ 2011-07-02 14:40       ` Nick Dokos
  2011-07-02 14:55         ` David Maus
  1 sibling, 1 reply; 10+ messages in thread
From: Nick Dokos @ 2011-07-02 14:40 UTC (permalink / raw)
  To: David Maus; +Cc: Bastien, emacs-orgmode, Paul Sexton, nicholas.dokos

David Maus <dmaus@ictsoc.de> wrote:


> #+begin_src emacs-lisp
>   (defmacro org-without-partial-completion (&rest body)
>     `(when (and (boundp 'partial-completion-mode)
>                 (fboundp 'partial-completion-mode))
>        (unwind-protect
>            (progn
>              (partial-completion-mode -1)
>              ,@body)
>          (partial-completion-mode 1))))
> #+end_src
> 
> This avoids leaking if 'body happens to uses a symbol 'pc-mode-p in a
> different context.
> 

Won't this turn on the mode even if it was off before the macro ws called?
And if so, isn't that a problem?

Nick

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-07-02 14:40       ` Nick Dokos
@ 2011-07-02 14:55         ` David Maus
  2011-07-03 10:44           ` Bastien
  0 siblings, 1 reply; 10+ messages in thread
From: David Maus @ 2011-07-02 14:55 UTC (permalink / raw)
  To: nicholas.dokos; +Cc: David Maus, emacs-orgmode, Paul Sexton, Bastien

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

At Sat, 02 Jul 2011 10:40:31 -0400,
Nick Dokos wrote:
>
> David Maus <dmaus@ictsoc.de> wrote:
>
>
> > #+begin_src emacs-lisp
> >   (defmacro org-without-partial-completion (&rest body)
> >     `(when (and (boundp 'partial-completion-mode)
> >                 (fboundp 'partial-completion-mode))
> >        (unwind-protect
> >            (progn
> >              (partial-completion-mode -1)
> >              ,@body)
> >          (partial-completion-mode 1))))
> > #+end_src
> >
> > This avoids leaking if 'body happens to uses a symbol 'pc-mode-p in a
> > different context.
> >
>
> Won't this turn on the mode even if it was off before the macro ws called?
> And if so, isn't that a problem?

Yes and yes.

(and (boundp 'partial-completion-mode)
     partial-completion-mode
     (fboundp 'partial-completion-mode))

Is the right condition. Bound, non-nil and callable.

@Bastien: Pushed fix for this to master.

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Typo in 'org-without-partial-completion'
  2011-07-02 14:55         ` David Maus
@ 2011-07-03 10:44           ` Bastien
  0 siblings, 0 replies; 10+ messages in thread
From: Bastien @ 2011-07-03 10:44 UTC (permalink / raw)
  To: David Maus; +Cc: emacs-orgmode, nicholas.dokos, Paul Sexton

Hi Nick, David,

David Maus <dmaus@ictsoc.de> writes:

> Yes and yes.
>
> (and (boundp 'partial-completion-mode)
>      partial-completion-mode
>      (fboundp 'partial-completion-mode))
>
> Is the right condition. Bound, non-nil and callable.
>
> @Bastien: Pushed fix for this to master.

Thanks!

-- 
 Bastien

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-07-03 10:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-22 21:08 Typo in 'org-without-partial-completion' Paul Sexton
2011-06-28 16:48 ` Bastien
2011-06-30  9:12 ` Bastien
2011-06-30 19:28   ` David Maus
2011-07-02  7:34     ` David Maus
2011-07-02  9:26       ` Bastien
2011-07-02 13:50         ` David Maus
2011-07-02 14:40       ` Nick Dokos
2011-07-02 14:55         ` David Maus
2011-07-03 10:44           ` Bastien

Code repositories for project(s) associated with this 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).