de-dynamicizing some code

de-dynamicizing some code

[Eric Abrahamsen]

I'm addressing this to Stefan, because of some comments he made
previously, but posting this here because it's probably of general
interest.

I'm working around some BBDB code that looks like this:

  (let* ((indent (or (bbdb-layout-get-option layout 'indentation) 21))
         (fmt (format " %%%ds: " (- indent 3)))
         start field formatfun)
    (dolist (field field-list)
      (setq start (point))
      (cond (;; customized formatting
             (setq formatfun (intern-soft (format "bbdb-display-%s-multi-line" field)))
             (funcall formatfun record))

What's happening is, you can provide a custom formatting function for a
field on a BBDB record, and BBDB will call the format function if it
exists.

The local vars `indent' and `fmt' are necessary for formatting the
field, and you access them dynamically from within the `formatfun'. This
makes compiler warnings, and is presumably bad for our lexical future.

I assume the proper solution is to replace this:

(funcall formatfun record)

with:

(funcall formatfun record indent fmt)

Is that correct?

Thanks,
Eric

Re: de-dynamicizing some code

[Stefan Monnier]

> I assume the proper solution is to replace this:
>      (funcall formatfun record)
> with:
>      (funcall formatfun record indent fmt)
> Is that correct?

There are various solutions, but this one looks like a very good
option, indeed.

The next best thing is to rename the variables to bbdb-fmt and
bbdb-indent (and to add "(defvar bbdb-indent) (defvar bbdb-fmt)"
somewhere at top-level (to tell the compiler that these vars should use
dynamic binding).


        Stefan

Re: de-dynamicizing some code

[Eric Abrahamsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I assume the proper solution is to replace this:
>>      (funcall formatfun record)
>> with:
>>      (funcall formatfun record indent fmt)
>> Is that correct?
>
> There are various solutions, but this one looks like a very good
> option, indeed.
>
> The next best thing is to rename the variables to bbdb-fmt and
> bbdb-indent (and to add "(defvar bbdb-indent) (defvar bbdb-fmt)"
> somewhere at top-level (to tell the compiler that these vars should use
> dynamic binding).

Interesting -- thanks. I guess in this case the simpler solution is
enough.

E