emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Problems with (defvar foo) and Emacs 23
@ 2012-04-01 18:57 Bernt Hansen
  2012-04-01 20:16 ` Achim Gratz
  2012-04-01 20:46 ` Nick Dokos
  0 siblings, 2 replies; 22+ messages in thread
From: Bernt Hansen @ 2012-04-01 18:57 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode

Hi Bastien,

I updated to master today e917477 ((org-xhtml.el): Removed, 2012-04-01)
and am getting errors about org-clock-last-state not defined in my GNU
Emacs 23.2.1 (i486-pc-linux-gnu, GTK+ Version 2.20.0) of 2010-12-11 on
raven, modified by Debian

I can see the variable in the source defined as

--8<---------------cut here---------------start------------->8---
lisp/org-clock.el:(defvar org-clock-state) ;; dynamically scoped into this function
--8<---------------cut here---------------end--------------->8---

but I don't get a variable definition with this code in emacs 23.2.1.

If I change the definition to

--8<---------------cut here---------------start------------->8---
(defvar org-clock-state nil)
--8<---------------cut here---------------end--------------->8---

then it works for me.

There are _lots_ of these types of definitions with no value in the
org-mode source.

--8<---------------cut here---------------start------------->8---
bernt@gollum:~/git/org-mode$ git grep '(defvar [a-z-]*)'  | wc -l
409
--8<---------------cut here---------------end--------------->8---

Regards,
Bernt

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 18:57 Problems with (defvar foo) and Emacs 23 Bernt Hansen
@ 2012-04-01 20:16 ` Achim Gratz
  2012-04-01 20:29   ` [URGENT] " Achim Gratz
                     ` (2 more replies)
  2012-04-01 20:46 ` Nick Dokos
  1 sibling, 3 replies; 22+ messages in thread
From: Achim Gratz @ 2012-04-01 20:16 UTC (permalink / raw)
  To: emacs-orgmode

Bernt Hansen writes:
> I can see the variable in the source defined as
>
> lisp/org-clock.el:(defvar org-clock-state) ;; dynamically scoped into this function
>
> but I don't get a variable definition with this code in emacs 23.2.1.

You aren't supposed to get one, as this should have been pulling in a
local variable defined elsewhere (from within another function).

> If I change the definition to
>
> (defvar org-clock-state nil)
>
> then it works for me.

Yes, but the bug introduced by renaming the variable is still there.
You do get a variable, but not the one you're supposed to be scoping.

> There are _lots_ of these types of definitions with no value in the
> org-mode source.

Again, the missing value is not the problem.  The problem arises when
the variable name in the caller and the callee becomes different.  I'll
let Bastien and Martyn sort that one out... :-)


Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

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

* Re: [URGENT] Problems with (defvar foo) and Emacs 23
  2012-04-01 20:16 ` Achim Gratz
@ 2012-04-01 20:29   ` Achim Gratz
  2012-04-02  6:15     ` Bastien
  2012-04-01 20:37   ` Bastien
  2012-04-02  6:12   ` Bastien
  2 siblings, 1 reply; 22+ messages in thread
From: Achim Gratz @ 2012-04-01 20:29 UTC (permalink / raw)
  To: emacs-orgmode

Achim Gratz writes:
> Again, the missing value is not the problem.  The problem arises when
> the variable name in the caller and the callee becomes different.  I'll
> let Bastien and Martyn sort that one out... :-)

The expedient fix would likely be to chose the same prefix for all the
others (dyn/org-…?), but all local bindings in all functions using these
would also need to change.  Since this is probably going to be a lot of
work, perhaps one could comment out those defvars were the variable
isn't actually global (like "state") and deal only with the remaining
ones if any.


HTH,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 20:16 ` Achim Gratz
  2012-04-01 20:29   ` [URGENT] " Achim Gratz
@ 2012-04-01 20:37   ` Bastien
  2012-04-01 21:18     ` Achim Gratz
  2012-04-02  6:12   ` Bastien
  2 siblings, 1 reply; 22+ messages in thread
From: Bastien @ 2012-04-01 20:37 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Again, the missing value is not the problem.  The problem arises when
> the variable name in the caller and the callee becomes different.  I'll
> let Bastien and Martyn sort that one out... :-)

Well -- I'll be pretty busy next week, so hopefully we can fix this
soon...  any help welcome!  :)

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 18:57 Problems with (defvar foo) and Emacs 23 Bernt Hansen
  2012-04-01 20:16 ` Achim Gratz
@ 2012-04-01 20:46 ` Nick Dokos
  2012-04-01 20:48   ` Bernt Hansen
  2012-04-02  6:09   ` Bastien
  1 sibling, 2 replies; 22+ messages in thread
From: Nick Dokos @ 2012-04-01 20:46 UTC (permalink / raw)
  To: Bernt Hansen; +Cc: nicholas.dokos, emacs-orgmode

Bernt Hansen <bernt@norang.ca> wrote:

> Hi Bastien,
> 
> I updated to master today e917477 ((org-xhtml.el): Removed, 2012-04-01)
> and am getting errors about org-clock-last-state not defined in my GNU
> Emacs 23.2.1 (i486-pc-linux-gnu, GTK+ Version 2.20.0) of 2010-12-11 on
> raven, modified by Debian
> 
> I can see the variable in the source defined as
> 
> lisp/org-clock.el:(defvar org-clock-state) ;; dynamically scoped into this function
> 
> but I don't get a variable definition with this code in emacs 23.2.1.
> 
> If I change the definition to
> 
> (defvar org-clock-state nil)
> 
> then it works for me.
> 
> There are _lots_ of these types of definitions with no value in the
> org-mode source.
> 

They are not supposed to *define* a variable. They are there to tell the
compiler not to worry. They are somewhat similar[fn:1] to extern declarations
in C code: whoever needs to use the variable says

  (defvar foo)

There is (supposed to be) *one* place somewhere that actually defines
it:

  (defvar foo 1)

Nick

Footnotes:

[fn:1] ...for some value of "similar". You have to take this with the
appropriately sized grain of salt.

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 20:46 ` Nick Dokos
@ 2012-04-01 20:48   ` Bernt Hansen
  2012-04-02  6:09   ` Bastien
  1 sibling, 0 replies; 22+ messages in thread
From: Bernt Hansen @ 2012-04-01 20:48 UTC (permalink / raw)
  To: nicholas.dokos; +Cc: emacs-orgmode

Nick Dokos <nicholas.dokos@hp.com> writes:

> Bernt Hansen <bernt@norang.ca> wrote:
>
>> Hi Bastien,
>> 
>> I updated to master today e917477 ((org-xhtml.el): Removed, 2012-04-01)
>> and am getting errors about org-clock-last-state not defined in my GNU
>> Emacs 23.2.1 (i486-pc-linux-gnu, GTK+ Version 2.20.0) of 2010-12-11 on
>> raven, modified by Debian
>> 
>> I can see the variable in the source defined as
>> 
>> lisp/org-clock.el:(defvar org-clock-state) ;; dynamically scoped into this function
>> 
>> but I don't get a variable definition with this code in emacs 23.2.1.
>> 
>> If I change the definition to
>> 
>> (defvar org-clock-state nil)
>> 
>> then it works for me.
>> 
>> There are _lots_ of these types of definitions with no value in the
>> org-mode source.
>> 
>
> They are not supposed to *define* a variable. They are there to tell the
> compiler not to worry. They are somewhat similar[fn:1] to extern declarations
> in C code: whoever needs to use the variable says
>
>   (defvar foo)
>
> There is (supposed to be) *one* place somewhere that actually defines
> it:
>
>   (defvar foo 1)
>
> Nick
>
> Footnotes:
>
> [fn:1] ...for some value of "similar". You have to take this with the
> appropriately sized grain of salt.

Ah, I had no idea how this was actually supposed to work :)
Thanks for the clarification.  I'll rewind to an older commit for now
that works for me ... since I need it at work tomorrow.

Thanks,
Bernt

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 20:37   ` Bastien
@ 2012-04-01 21:18     ` Achim Gratz
  2012-04-02  0:53       ` Nick Dokos
  2012-04-02  6:27       ` Bastien
  0 siblings, 2 replies; 22+ messages in thread
From: Achim Gratz @ 2012-04-01 21:18 UTC (permalink / raw)
  To: emacs-orgmode

Bastien writes:
> Well -- I'll be pretty busy next week, so hopefully we can fix this
> soon...  any help welcome!  :)

Ditto, but you do realize this will be horribly broken in Emacs 24?

Anyway, for that single dynamic "state" variable: it is let-bound in
org.el/org-todo and then dynamically scoped in many, many places:

contrib/lisp/{org-{checklist,choose},org2rem}.el

/lisp/{org-{agenda,clock,icalendar,mouse,taskjuggler},org}.el

So the correct prefixed name should probably be org-todo-state (there
are other such "state"s in other places, don't know yet if they are also
dynamically scoped into other functions).  You need to also keep track
of which functions use "state" as a formal parameter name, since these
shadow the dynamic variable from the outside, but provide another
"state" for calls on the inside…

Haven't yet checked any of the other definitions that had their name
changed, gotta fetch some sleep before work.


HTH,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 21:18     ` Achim Gratz
@ 2012-04-02  0:53       ` Nick Dokos
  2012-04-02  5:31         ` Achim Gratz
  2012-04-02  6:27       ` Bastien
  1 sibling, 1 reply; 22+ messages in thread
From: Nick Dokos @ 2012-04-02  0:53 UTC (permalink / raw)
  To: Bastien; +Cc: Achim Gratz, nicholas.dokos, emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> wrote:

> Bastien writes:
> > Well -- I'll be pretty busy next week, so hopefully we can fix this
> > soon...  any help welcome!  :)
> 
> Ditto, but you do realize this will be horribly broken in Emacs 24?
> 
> Anyway, for that single dynamic "state" variable: it is let-bound in
> org.el/org-todo and then dynamically scoped in many, many places:
> 
> contrib/lisp/{org-{checklist,choose},org2rem}.el
> 
> /lisp/{org-{agenda,clock,icalendar,mouse,taskjuggler},org}.el
> 
> So the correct prefixed name should probably be org-todo-state (there
> are other such "state"s in other places, don't know yet if they are also
> dynamically scoped into other functions).  You need to also keep track
> of which functions use "state" as a formal parameter name, since these
> shadow the dynamic variable from the outside, but provide another
> "state" for calls on the inside…
> 
> Haven't yet checked any of the other definitions that had their name
> changed, gotta fetch some sleep before work.
> 
> 

I assume that we are talking about the seven commits

$ git log --oneline -100 | grep 'Fix global'
6cbf1f4 Fix global dynamic variables in org-agenda.el and org.el.
b689cbf Fix global dynamic variables in org-table.el.
9054ba3 Fix global dynamic variables in org-special-blocks.el.
b46fa17 Fix global dynamic variables in org-clock.el.
08d9b46 Fix global dynamic variables prefixes in org-bibtex.el.
c24fa19 Fix global dynamic variables prefixes in org-mouse.el.
fcf13e0 Fix global dynamic variables prefixes in org-beamer.el.

I tried reverting these in a branch: I had to merge lisp/org.el by hand
while reverting the first one, but the rest went through without any
problems.

Does that seem like the right thing to do for now then?

Nick

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-02  0:53       ` Nick Dokos
@ 2012-04-02  5:31         ` Achim Gratz
  2012-04-02  6:01           ` Nick Dokos
  2012-04-02  6:28           ` Bastien
  0 siblings, 2 replies; 22+ messages in thread
From: Achim Gratz @ 2012-04-02  5:31 UTC (permalink / raw)
  To: emacs-orgmode

Nick Dokos writes:
> I assume that we are talking about the seven commits

Yes, but likely not all of them (or all changes in them).  The only
problematic ones are where a dynamically scoped variable is renamed and
there are a bunch of others that probably are OK.

> $ git log --oneline -100 | grep 'Fix global'
> 6cbf1f4 Fix global dynamic variables in org-agenda.el and org.el.
> b689cbf Fix global dynamic variables in org-table.el.
> 9054ba3 Fix global dynamic variables in org-special-blocks.el.
> b46fa17 Fix global dynamic variables in org-clock.el.
> 08d9b46 Fix global dynamic variables prefixes in org-bibtex.el.
> c24fa19 Fix global dynamic variables prefixes in org-mouse.el.
> fcf13e0 Fix global dynamic variables prefixes in org-beamer.el.
>
> I tried reverting these in a branch: I had to merge lisp/org.el by hand
> while reverting the first one, but the rest went through without any
> problems.
>
> Does that seem like the right thing to do for now then?

For Emacs 24, it is probably the right thing to do, depending on how
much time there is left before release.  In hotfix/master the changes
should be corrected so that they do what they were supposed to do
(i.e. also rename the variables in the caller).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-02  5:31         ` Achim Gratz
@ 2012-04-02  6:01           ` Nick Dokos
  2012-04-02  8:21             ` Bastien
  2012-04-02  6:28           ` Bastien
  1 sibling, 1 reply; 22+ messages in thread
From: Nick Dokos @ 2012-04-02  6:01 UTC (permalink / raw)
  To: Achim Gratz; +Cc: nicholas.dokos, emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> wrote:

> Nick Dokos writes:
> > I assume that we are talking about the seven commits
> 
> Yes, but likely not all of them (or all changes in them).  The only
> problematic ones are where a dynamically scoped variable is renamed and
> there are a bunch of others that probably are OK.
> 

Agreed, but the point is that each and every variable renaming will need
to be checked in the light of these criteria. Bugs like this have the
potential of creating havoc for a long time to come.

It may be easier to start from a working state than to try to fix the
current broken state piecemeal. Besides, I'd rather have a single commit
in the history that does the right thing: having one commit that creates
the initial buggy state and then N commits at various times to fix the
brokenness might create all sorts of problems (breaking bisectability
e.g.)

Nick

> > $ git log --oneline -100 | grep 'Fix global'
> > 6cbf1f4 Fix global dynamic variables in org-agenda.el and org.el.
> > b689cbf Fix global dynamic variables in org-table.el.
> > 9054ba3 Fix global dynamic variables in org-special-blocks.el.
> > b46fa17 Fix global dynamic variables in org-clock.el.
> > 08d9b46 Fix global dynamic variables prefixes in org-bibtex.el.
> > c24fa19 Fix global dynamic variables prefixes in org-mouse.el.
> > fcf13e0 Fix global dynamic variables prefixes in org-beamer.el.
> >
> > I tried reverting these in a branch: I had to merge lisp/org.el by hand
> > while reverting the first one, but the rest went through without any
> > problems.
> >
> > Does that seem like the right thing to do for now then?
> 
> For Emacs 24, it is probably the right thing to do, depending on how
> much time there is left before release.  In hotfix/master the changes
> should be corrected so that they do what they were supposed to do
> (i.e. also rename the variables in the caller).
> 
> 
> Regards,
> Achim.
> -- 
> +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+
> 
> Wavetables for the Waldorf Blofeld:
> http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables
> 
> 

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 20:46 ` Nick Dokos
  2012-04-01 20:48   ` Bernt Hansen
@ 2012-04-02  6:09   ` Bastien
  1 sibling, 0 replies; 22+ messages in thread
From: Bastien @ 2012-04-02  6:09 UTC (permalink / raw)
  To: nicholas.dokos; +Cc: Bernt Hansen, emacs-orgmode

Nick Dokos <nicholas.dokos@hp.com> writes:

> There is (supposed to be) *one* place somewhere that actually defines
> it:
>
>   (defvar foo 1)

... unless foo is dynamically bound into a "caller" sexp.

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 20:16 ` Achim Gratz
  2012-04-01 20:29   ` [URGENT] " Achim Gratz
  2012-04-01 20:37   ` Bastien
@ 2012-04-02  6:12   ` Bastien
  2012-04-07 12:15     ` Matt Lundin
  2 siblings, 1 reply; 22+ messages in thread
From: Bastien @ 2012-04-02  6:12 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Bernt Hansen writes:
>> I can see the variable in the source defined as
>>
>> lisp/org-clock.el:(defvar org-clock-state) ;; dynamically scoped into this function
>>
>> but I don't get a variable definition with this code in emacs 23.2.1.
>
> You aren't supposed to get one, as this should have been pulling in a
> local variable defined elsewhere (from within another function).
>
>> If I change the definition to
>>
>> (defvar org-clock-state nil)
>>
>> then it works for me.
>
> Yes, but the bug introduced by renaming the variable is still there.
> You do get a variable, but not the one you're supposed to be scoping.

I fixed the problem with `org-clock-state'.  This should be `org-state'.

`state' is a local variable in many org.el functions, I renamed it to
`org-state' in org-clock.el and in "caller" sexp from org.el.  

> Again, the missing value is not the problem.  The problem arises when
> the variable name in the caller and the callee becomes different.  I'll
> let Bastien and Martyn sort that one out... :-)

Let's sort this out by slowly fixing problems that we can spot.

-- 
 Bastien

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

* Re: [URGENT] Problems with (defvar foo) and Emacs 23
  2012-04-01 20:29   ` [URGENT] " Achim Gratz
@ 2012-04-02  6:15     ` Bastien
  0 siblings, 0 replies; 22+ messages in thread
From: Bastien @ 2012-04-02  6:15 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Achim Gratz writes:
>> Again, the missing value is not the problem.  The problem arises when
>> the variable name in the caller and the callee becomes different.  I'll
>> let Bastien and Martyn sort that one out... :-)
>
> The expedient fix would likely be to chose the same prefix for all the
> others (dyn/org-…?), 

We need to use org- as a prefix.  org-dyn is a good candidate but let's
check in other packages if there is a convention about this.

> but all local bindings in all functions using these
> would also need to change.  Since this is probably going to be a lot of
> work, perhaps one could comment out those defvars were the variable
> isn't actually global (like "state") and deal only with the remaining
> ones if any.

Not sure I understand (it's quite early in the morning...) - can you
explain this?

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-01 21:18     ` Achim Gratz
  2012-04-02  0:53       ` Nick Dokos
@ 2012-04-02  6:27       ` Bastien
  2012-04-02 18:11         ` Achim Gratz
  1 sibling, 1 reply; 22+ messages in thread
From: Bastien @ 2012-04-02  6:27 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Bastien writes:
>> Well -- I'll be pretty busy next week, so hopefully we can fix this
>> soon...  any help welcome!  :)
>
> Ditto, but you do realize this will be horribly broken in Emacs 24?

http://orgmode.org/w/?p=org-mode.git;a=commitdiff;h=08d9b466225fad7e4cacd593f5ec7d2a4cd878ff
http://orgmode.org/w/?p=org-mode.git;a=commitdiff;h=c24fa194a62cbbc45fa1b58e39dac293a006a0c9
http://orgmode.org/w/?p=org-mode.git;a=commitdiff;h=fcf13e02aa4bd056316cb7476b94aecf80738b47
http://orgmode.org/w/?p=org-mode.git;a=commitdiff;h=9054ba39d085dc2910285a194ed2206b36875289
http://orgmode.org/w/?p=org-mode.git;a=commitdiff;h=b689cbfb6c4d287d839ac3b0727497e5114995d8

are fine.

http://orgmode.org/w/?p=org-mode.git;a=commitdiff;h=b46fa17a97ee050b5aeccffaa7323201fe491371

was problematic, as reported by Bernt, and I think I fixed it.

It leaves us with 

http://orgmode.org/w/?p=org-mode.git;a=commitdiff;h=6cbf1f417222321a47848a7368427ba8a22fe3a5

which should be carefully reviewed/tested/amended.

> Anyway, for that single dynamic "state" variable: it is let-bound in
> org.el/org-todo and then dynamically scoped in many, many places:
>
> contrib/lisp/{org-{checklist,choose},org2rem}.el
>
> /lisp/{org-{agenda,clock,icalendar,mouse,taskjuggler},org}.el

The `state' dynamically scoped in these files is not necessarily the
same `state' that is dynamically scoped in `org-todo'.  

For example in org-agenda.el there is `state' in `org-agenda-log-mode',
but this is dynamically scoped in another function org-agenda.el that
does not depend on org.el's definition for `state'...

> So the correct prefixed name should probably be org-todo-state (there
> are other such "state"s in other places, don't know yet if they are also
> dynamically scoped into other functions).  You need to also keep track
> of which functions use "state" as a formal parameter name, since these
> shadow the dynamic variable from the outside, but provide another
> "state" for calls on the inside…

Yep -- I guess we're good for a fun ride.

> Haven't yet checked any of the other definitions that had their name
> changed, gotta fetch some sleep before work.

Please do!

Let's do some heavy testing against current HEAD and let's carefully 
review code.  But let's not panic -- Emacs is still in pretest, that's
the whole purpose of a pretest.  

Thanks all for your help,

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-02  5:31         ` Achim Gratz
  2012-04-02  6:01           ` Nick Dokos
@ 2012-04-02  6:28           ` Bastien
  1 sibling, 0 replies; 22+ messages in thread
From: Bastien @ 2012-04-02  6:28 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Nick Dokos writes:
>> I assume that we are talking about the seven commits
>
> Yes, but likely not all of them (or all changes in them).  The only
> problematic ones are where a dynamically scoped variable is renamed and
> there are a bunch of others that probably are OK.

Let's have a particular look at the renaming of entry->org-entry.

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-02  6:01           ` Nick Dokos
@ 2012-04-02  8:21             ` Bastien
  0 siblings, 0 replies; 22+ messages in thread
From: Bastien @ 2012-04-02  8:21 UTC (permalink / raw)
  To: nicholas.dokos; +Cc: Achim Gratz, emacs-orgmode

Hi Nick,

Nick Dokos <nicholas.dokos@hp.com> writes:

> Agreed, but the point is that each and every variable renaming will need
> to be checked in the light of these criteria. Bugs like this have the
> potential of creating havoc for a long time to come.
>
> It may be easier to start from a working state than to try to fix the
> current broken state piecemeal.

The thing is...  Org is working fine here.  Of course, I'm not a
"thousand eyes" by myself and some features might be broken in the
current state, I will continue trying to catch them.

> Besides, I'd rather have a single commit
> in the history that does the right thing: having one commit that creates
> the initial buggy state and then N commits at various times to fix the
> brokenness might create all sorts of problems (breaking bisectability
> e.g.)

Theoretically agreed.  But again: if you look at the diffs from the 
list of commits you sent, most of them are okay.  Let's try to fix the
ones that are *not* okay instead of going backward?  I will revert the 
commits if things are really broken.

Best,

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-02  6:27       ` Bastien
@ 2012-04-02 18:11         ` Achim Gratz
  2012-04-03  5:49           ` Bastien
  0 siblings, 1 reply; 22+ messages in thread
From: Achim Gratz @ 2012-04-02 18:11 UTC (permalink / raw)
  To: emacs-orgmode

Bastien writes:
> Let's do some heavy testing against current HEAD and let's carefully 
> review code.  But let's not panic -- Emacs is still in pretest, that's
> the whole purpose of a pretest.  

It looks like you've already took care of the fixes.  Anything that you
specifically want to review?  The TODO stuff would not escape Bernt's
scrutiny, I suspect...


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-02 18:11         ` Achim Gratz
@ 2012-04-03  5:49           ` Bastien
  2012-04-03  6:04             ` Achim Gratz
  0 siblings, 1 reply; 22+ messages in thread
From: Bastien @ 2012-04-03  5:49 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Hi Achim,

Achim Gratz <Stromeko@nexgo.de> writes:

> Bastien writes:
>> Let's do some heavy testing against current HEAD and let's carefully 
>> review code.  But let's not panic -- Emacs is still in pretest, that's
>> the whole purpose of a pretest.  
>
> It looks like you've already took care of the fixes.  Anything that you
> specifically want to review?  The TODO stuff would not escape Bernt's
> scrutiny, I suspect...

There are these warnings left while compiling Org in Emacs 24:

1. one about `buffer-substring-filters'
2. one about `entry' not having a prefix
3. one about `date', `annotation', 'initial' not having a prefix

We should write a compatibility function to get rid of the first
warning.

Prefixing `entry' is not straightforward but should be possible.

As for the last warnings, I think we'll have to live with it,
but chasing down places where *some* instance can be prefixed
(or renamed with a more explicit name) could be good.  Not a
high priority though.

Nothing here is critical right now -- thanks for your help!

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-03  5:49           ` Bastien
@ 2012-04-03  6:04             ` Achim Gratz
  2012-04-04  7:25               ` Bastien
  0 siblings, 1 reply; 22+ messages in thread
From: Achim Gratz @ 2012-04-03  6:04 UTC (permalink / raw)
  To: emacs-orgmode

Bastien writes:
> 1. one about `buffer-substring-filters'
> We should write a compatibility function to get rid of the first
> warning.

Actually it is a variable and it "just" needs to be aliased suitably,
depending on which Emacs version it encounters.  This should be done
with a macro in org-compat.el (which doesn't exist yet, see my attempt
at such a macro in another thread), then org can use the aliased name
only.  In general I'd say org-compat.el is in need of some love.

> 2. one about `entry' not having a prefix
> Prefixing `entry' is not straightforward but should be possible.

I don't know enough about its uses to help here.

> 3. one about `date', `annotation', 'initial' not having a prefix
> As for the last warnings, I think we'll have to live with it,
> but chasing down places where *some* instance can be prefixed
> (or renamed with a more explicit name) could be good.  Not a
> high priority though.

I think one could also alias these (the prefix should be that of the
package where they come from), so that all uses within org use that
prefix.  That would probably leave one warning at the point where the
alias is defined, which could be wrapped into with-no-warning.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-03  6:04             ` Achim Gratz
@ 2012-04-04  7:25               ` Bastien
  0 siblings, 0 replies; 22+ messages in thread
From: Bastien @ 2012-04-04  7:25 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Bastien writes:
>> 1. one about `buffer-substring-filters'
>> We should write a compatibility function to get rid of the first
>> warning.
>
> Actually it is a variable and it "just" needs to be aliased suitably,
> depending on which Emacs version it encounters.  

I'm afraid this is not that straightforward, because the use of
`filter-buffer-substring-functions' and `buffer-substring-filters'
differ.

> This should be done
> with a macro in org-compat.el (which doesn't exist yet, see my attempt
> at such a macro in another thread), then org can use the aliased name
> only.  In general I'd say org-compat.el is in need of some love.

Make yourself at home there :)
>
>> 2. one about `entry' not having a prefix
>> Prefixing `entry' is not straightforward but should be possible.
>
> I don't know enough about its uses to help here.

No problem.

>> 3. one about `date', `annotation', 'initial' not having a prefix
>> As for the last warnings, I think we'll have to live with it,
>> but chasing down places where *some* instance can be prefixed
>> (or renamed with a more explicit name) could be good.  Not a
>> high priority though.
>
> I think one could also alias these (the prefix should be that of the
> package where they come from), so that all uses within org use that
> prefix.  That would probably leave one warning at the point where the
> alias is defined, which could be wrapped into with-no-warning.

Good idea.  When you have time for a patch to testing this solution, 
let me know.

Thanks!

-- 
 Bastien

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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-02  6:12   ` Bastien
@ 2012-04-07 12:15     ` Matt Lundin
  2012-04-09 14:57       ` Bastien
  0 siblings, 1 reply; 22+ messages in thread
From: Matt Lundin @ 2012-04-07 12:15 UTC (permalink / raw)
  To: Bastien; +Cc: Achim Gratz, emacs-orgmode

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

Bastien <bzg@gnu.org> writes:
>
> I fixed the problem with `org-clock-state'.  This should be `org-state'.
>
> `state' is a local variable in many org.el functions, I renamed it to
> `org-state' in org-clock.el and in "caller" sexp from org.el.  
>

Attached please find a patch that fixes the docstring of
org-after-todo-state-change-hook to reflect this change.

Best,
Matt


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-hook-docstring-to-reflect-renaming-of-state-to-o.patch --]
[-- Type: text/x-patch, Size: 1061 bytes --]

From f21889985f2008bc77e2ecfc38be481a47c9916c Mon Sep 17 00:00:00 2001
From: Matt Lundin <mdl@imapmail.org>
Date: Sat, 7 Apr 2012 07:13:55 -0500
Subject: [PATCH] Fix hook docstring to reflect renaming of state to
 org-state.

* lisp/org.el: (org-after-todo-state-change-hook): Fix docstring to
  reflect name change of state to org-state.

The renamed variable can cause user hooks added to
org-after-todo-state-change-hook to break, so it is essential to have
the correct variable name in the docstring.
---
 lisp/org.el |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index a11d734..e7c579d 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2231,7 +2231,7 @@ property and include the word \"recursive\" into the value."
 (defcustom org-after-todo-state-change-hook nil
   "Hook which is run after the state of a TODO item was changed.
 The new state (a string with a TODO keyword, or nil) is available in the
-Lisp variable `state'."
+Lisp variable `org-state'."
   :group 'org-todo
   :type 'hook)
 
-- 
1.7.10


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

* Re: Problems with (defvar foo) and Emacs 23
  2012-04-07 12:15     ` Matt Lundin
@ 2012-04-09 14:57       ` Bastien
  0 siblings, 0 replies; 22+ messages in thread
From: Bastien @ 2012-04-09 14:57 UTC (permalink / raw)
  To: Matt Lundin; +Cc: Achim Gratz, emacs-orgmode

Matt Lundin <mdl@imapmail.org> writes:

> Bastien <bzg@gnu.org> writes:
>>
>> I fixed the problem with `org-clock-state'.  This should be `org-state'.
>>
>> `state' is a local variable in many org.el functions, I renamed it to
>> `org-state' in org-clock.el and in "caller" sexp from org.el.  
>
> Attached please find a patch that fixes the docstring of
> org-after-todo-state-change-hook to reflect this change.

Applied, thanks!

-- 
 Bastien

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

end of thread, other threads:[~2012-04-09 14:56 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-01 18:57 Problems with (defvar foo) and Emacs 23 Bernt Hansen
2012-04-01 20:16 ` Achim Gratz
2012-04-01 20:29   ` [URGENT] " Achim Gratz
2012-04-02  6:15     ` Bastien
2012-04-01 20:37   ` Bastien
2012-04-01 21:18     ` Achim Gratz
2012-04-02  0:53       ` Nick Dokos
2012-04-02  5:31         ` Achim Gratz
2012-04-02  6:01           ` Nick Dokos
2012-04-02  8:21             ` Bastien
2012-04-02  6:28           ` Bastien
2012-04-02  6:27       ` Bastien
2012-04-02 18:11         ` Achim Gratz
2012-04-03  5:49           ` Bastien
2012-04-03  6:04             ` Achim Gratz
2012-04-04  7:25               ` Bastien
2012-04-02  6:12   ` Bastien
2012-04-07 12:15     ` Matt Lundin
2012-04-09 14:57       ` Bastien
2012-04-01 20:46 ` Nick Dokos
2012-04-01 20:48   ` Bernt Hansen
2012-04-02  6:09   ` Bastien

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