vm team mailing list archive

[Tim Cross <tcross@xxxxxxxxxxxxxxx>]

From: Uday S Reddy <u.s.reddy@xxxxxxxxxxxxx>
Subject: Re: [Vm] compilation warnings
Date: Sat, 3 Jul 2010 11:48:39 +0100

> Tim Cross writes:
> 
>> The focus-frame ones were fairly straight-forward. The existing calls were
>> doing nothing, but from the code, the objective seemed to be to make the frame
>> take focus, so I added the appropriate call to do this. 
> 
> Will it work on the supported older versions of emacsen?

I believe it does on emacs 22.1 onwards. 

> 
>> All but one of the default-enable-multibyte-characters were quite
>> straight-forward. The one in vm-read-folder was a bit different. In most
>> cases, the call was inside a let and used to set the value before creating the
>> buffer. In this case, I wrapped the original in a test for boundp and then,
>> inside the excursion where the buffer was made current, called
>> set-buffer-multibyte. 
> 
> But the doc-string for default-enable-multibyte-characters says one
> should use enable-multibyte-characters.  Have you tried that?

If you look at enable-multibyte-characters, it says NOT to set that directly,
but to use set=buffer-multibyte to set it.

>From the doc string of enable-multibyte-characters -

This variable is buffer-local but you cannot set it directly;
use the function `set-buffer-multibyte' to change a buffer's representation.
Changing its default value with `setq-default' is supported.
See also variable `default-enable-multibyte-characters' and Info node
`(elisp)Text Representations'.

> 
> In any case, the multibyte stuff is delicate and can cause serious
> folder corruption unless done correctly.
> 
> So, we better leave this to Ulrich to look at.
> 
>> In the case of vm-read-folder, I just commented out the offending line as I
>> don't think it is necessary. That function is only called by the top level
>> 'vm' function and that function does an explicit set-buffer-multibyte anyway. 
> 
> I guess this is part of the multibyte stuff?  If so, we leave it to
> Ulrich.
> 
> But note that this kind of reasoning can be problematic.
> vm-read-folder should do whatever it is logically required to do,
> without depending on what the callers do.

I would agree, but to be honest, the caller in this case is doing a lot more
wrt multibyte settings and more comprehensively than vm-read-folder, including
taking into consideration emacs/xemacs differences in this area. Although I've
not looked closer, but was surprised that the top level 'vm' function was the
only one calling this funnction. I would have expected a function called
vm-read-folder would have been called whenever you visited a folder.

> 
>> Code is compiling without these warnings and I've done a few tests (reading in
>> new folders, deleting/expunging from folders etc, and it all appears to work.
>> I'm testing with emacs 23.2. 
> 
> In these kinds of situations, we also need to make sure that all the
> changed code is exercised by the tests.  That can be quite hard.  I
> wish we had some tools for checking the coverage of tests.
> 

Yes, test coverage would be really good. However, from experience, adding or
incorporating such tools into an existing package with so much legacy code is
extremely difficult. It could be done, but will take a massive amount of
time/effort. 

Tim