vm team mailing list archive

[Tim Cross <tcross@xxxxxxxxxxxxxxx>]

From: Uday S Reddy <u.s.reddy@xxxxxxxxxxxxx>
Subject: Re: VM custom definitions
Date: Sun, 20 Jun 2010 22:12:18 +0100

> Tim Cross writes:
> 
>> Don't misunderstand, I'm not talking about wholesale
>> re-organisation. However, I do think the code should be getting
>> better as we maintain it. It needs to be left in a better state than
>> we find it. The things I'm looking at are quite straight-forward,
>> even trivial. Note also that we are talking only about a very small
>> number of these defcustom definitions. 
> 
> If it is only a small number of defcustom definitions, then I am not
> sure why we need such a big discussion about it.  In fact, I just went
> to see how many core VM defcustom's are outside vm-vars.el, and found
> only two.  So, just go ahead and move them.  It is not a big deal.

I didn't think it was a big deal. I would have just gone ahead and done it,
but as I wasn't sure about previous plans or discussions and being new to
contributing, thought it was good to get direction rather than just go off an
do it.

There are a total of 111 defcustom definitions that are not in vm-vars.el.
Many of these are for 'add-ons', but there are also a number of defcustom
definitions for add-ons that are already in vm-vars.el 

In 'core' packages (from current vm trunk) we have the following definitions
that are not in vm-vars.el

3 in vm-w3m.el
1 in vm-toolbar.el
11 in vm-ps-print.el
1 in vm-message.el
1 in vm-mime.el

All of these, with the possible exception of vm-w3m.el already have some, if
not a majority, of their defcustom forms in vm-vars.el

All I wanted to do was make this more consistent. As I stated in the original
post, I don't have a preference other than being consistent. I  would suggest
the following -

1. vm-var.el only contains defcustom/defface definitions for user customizable
options relating to core VM features. If this results in any build-errors, fix
them. 

2. Any defcustom/defface/defgroup related to add-ons live in the add-on files. 

Separating the two will ensure that when running only core features, you won't
also see non-core options when using customize. This is probably important as
it could cause confusion.

3. Remove any duplicate defgroup/defcustom/defface forms.

4. Fix any 'mismatch' errors between defcustom type definitions and default
values. 

5. Make use of face definitions consistent. Instead of a defcustom form that
specifies a face symbol, use a proper defface form, allowing consistent
customization of faces.

6. Fix documentation so that it is correct and clear. 

7, Use :link et. al. to reference the appropriate sections in the VM manual
for core options. 

8. Ensure custom definitions are loaded correctly. Currently, you get a
mish-mash of options depending on whether you have loaded things yet or not.
For example, you only get 3 of the vm-ps-print options unless you have
used/loaded vm-ps-print. Once you do use vm-ps-print, you get the full 11
options next time you run custom. 

Keep in mind that all of this is going into its own branch. It will be very
easy to merge it in if the results look good an pass peer assessment and
testing. Even if we later want to remove the changes, it isn't that hard to
revert things.

> 
>> My approach is to keep changes to a minimum. 
> 
> Good. That is what I am recommending as well. 
> 
>> Currently, there are problems with duplicate definitions of some
>> variables and custom groups. There are custom definitions that are
>> later completely ignored because of (setq blah) lines in a different
>> file and even in the core vm stuff, such as vm-toolbar.el, some of
>> the defcustom definitions are in vm-vars.el and some are in
>> vm-toolbar.el.
> 
> vm-toolbar.el has only one defcustom definition and it is not
> duplicated in vm-vars.el.  So, I am not sure what you are after.  Can
> you please be more specific when you make proposals, so that we can
> move faster in discussions?
>

There are 5 defcustom forms relating to the toolbar. All but one of them is in
vm-toolbar.el. The rest are in vm-vars.el
 
>> There are even custom definitions where the form of
>> the defcustom and the default value it is set to are not consistent
>> and we have things like some faces that are defined as 'real' face
>> and defface values and other faces that are defined as defcusotm
>> variables holding face names etc. While things like tag files can
>> help, they don't if you have multiple definitions of the same
>> thing. We even have the same variable defined twice in the same file
>> (vm-vars.el).
> 
> If there are problems like multiple definitions for the same thing, we
> should definitely fix them.  But, please tell us what these problems
> are, specifically.
>

Specifically

1. Multiple defgroup definitions for the vm group
2. Multiple defcustom definitions for the same variable
3. Inconsistent use of defface/defcustom, which also results in inconsistent
code. i.e. in some places we have a real face definition that is referred to
by symbol name and in other cases we have variables that hold the face name.
So we have code like

   (if vm-some-face
     (vm-summarise-region start end vm-some-face)
    (vm-summarise-region start end 'vm-some-other-face))

In addition, some faces you can customize in a standard face customization
manner and other faces, you can only customize the face by selecting the name
of an existing face (which the end user needs to know). 

3. setq statements on a variable, totally defeating the defcusotm or any user
.vm customization.

4. Poor or even incorrect defcustom definitions i.e. the defcustom type does
not match with the default setting (resulting in the message "STANDARD
(mismatch) when viewing the custom entry) or just poor/misleading definition,
making it harder for the end user to 'do the right thing'.

5. Inconsistent loading of custom options. What you get will depend on whether
you have run VM and what options you have used when you do.
 
>> The fact we have these inconsistencies is likely a significant cause
>> of maintenance problems and subtle bugs.
> 
> The consistency that I would like is that if I go to look up the
> variables or custom's of a particular kind, say vm-pop, then I would
> prefer to find all of them in one place.  Whether they are in
> vm-vars.el or vm-pop.el doesn't concern me too much.  
>

Exactly my point. This is currently not the case.
 
>> Your point regarding core and none-core VM stuff is a valid one and
>> partially why I asked for direction. I personally would work towards
>> no non-core definitions being in vm-vars.el. While they have been
>> put in there at different times to get around issues in building
>> etc, I would prefer that we actually fix such issues rather than
>> paper over them. 
> 
> Yes, anything you can do to separate the core VM from the add-ons
> would be very welcome.
> 
>> At some point, if we really want to maintain both a stable and
>> extensible MUA we are going to have to address these matters. Some
>> of the contributed stuff has significant problems, some of it is
>> actually better than what is in VN core. At some point we have to
>> work out what to do about this kludge of features and extensions and
>> work out how to integrate into VM proper. We should not be afraid to
>> re-factor and improve things. We do need to ensure quality and
>> stability, but we can do this through care, peer review and good
>> testing. 
> 
> All that sounds very good.  The real limitation we have is that we are
> only a few people who have other real jobs and we can only devote so
> much time for this.  
> 
> I think Rob F was wanting to integrate all the add-on's into the VM
> core, but he didn't actually manage to do very much about it.  So, we
> are left with a hodge-podge of stuff in the VM distribution.  I have
> integrated a few features from vm-rfaddons, but found it to be too
> much work.  So, I am only going to do such integration for the
> features that I think are really important.
>

It is a big job and not one that will be done in a single release. We can
probably achieve quite a lot just doing small bits at a time. However, your
correct that many parts will be difficult to integrate well. For example, I
like using the vm-summary-faces add on. However, it does conflict/duplicate
some of the stuff in core vm (not in a major way) and some of the stuff in
vm-rfaddons, not to mention uv-vm-colors.el
 
>> If we continue to keep tacking on bits and new features we
>> will just end up with a Frankenstein! The fact we distribute these
>> non-core bits with the main distribution I think also undermines our
>> position somewhat. End users are not going to make the
>> distinction. They download the tar ball or install the distro
>> packaged version and expect it to work because we have contributed
>> it with the release.
> 
> Sorry, I don't agree.  The VM core is solid, and all the new features
> that I am adding or I am overseeing are integrated properly.  No
> Frankenstein here.  
>

We will have to disagree I guess. 
 
> The VM core is what is described in the VM manual and the end users
> will need to learn the distinction between it and the add-ons, if they
> haven't figured it out already.  
> 

I find this statement surprising given your other comments regarding users not
doing the right thing with respect to reporting bugs or trying dev systems
etc.  I guess we will find out how it goes over time.

Tim