Jean-Marc Lasgouttes wrote:
> Actually, now that I have applied it, I see it does not look so good
I'm always happy to learn...
> A few things:
> fl_set_choice_text(class_->choice_doc_fonts,
> params.fonts.c_str());
>
> This one belongs to class_update
Yes, sorry. This one went in by accident.
> fl_clear_choice(class_->choice_doc_fontsize);
> fl_addto_choice(class_->choice_doc_fontsize, "default");
> fl_addto_choice(class_->choice_doc_fontsize,
> tclass.opt_fontsize().c_str());
> fl_set_choice(class_->choice_doc_fontsize,
> tokenPos(tclass.opt_fontsize(), '|',
> params.fontsize)+2);
>
> As I already wrote, fl_set_choice_text would be better here.
OK. Changed (though it was not my code ;-)).
> However,
> I think in fact that this last thing belongs to class_update (or at
> least should be duplicated in class_update).
You mean the fl_set_choice_text stuff? We need this in UpdateClassParams,
otherwise the choices will be reset to "default" after you have selected
another class in the combox (and we don't want to reset anything at that
point of time). If we have this in UpdateClassParams, however, I don't see
why we should duplicate it in class_update.
> In the later case
> class_update would not have to call UpdateClassParams.
This will not work. You need the methods that we have moved to
UpdateClassParams. Else the fontsizes will not be read in correctly e.g. if
you do: change from article to AMSart, _without_ applying: cancel, reopen the
dialog -> the AMSart fontsizes are read in for article.
> In the former
> case, you could only pass the params.textclass as parameter instead of
> the whole params structure. This shows better what you intend to do.
I do not understand this, sorry.
> void FormDocument::class_update(BufferParams const & params)
> {
> if (!class_.get())
> return;
>
> LyXTextClass const & tclass = textclasslist[params.textclass];
>
> combo_doc_class->select(tclass.description());
>
> If you really call UpdateClassParams below, then the select above is
> duplicated. So if you remove it, no need to define tclass.
>
> UpdateClassParams(params);
OK. I see.
> The rest looks OK.
>
> I do not have much time to modify and test properly this, so I'd
> appreciate an updated patch :)
> JMarc
Please have a look at the following code part. Does this get closer to your
plan?
Thanks,
Juergen.
void FormDocument::UpdateClassParams(BufferParams const & params)
{
// These are the params that have to be updated on any class change
// (even if the class defaults are not used) (JSpitzm 2002-04-08)
LyXTextClass const & tclass = textclasslist[params.textclass];
combo_doc_class->select(tclass.description());
fl_clear_choice(class_->choice_doc_fontsize);
fl_addto_choice(class_->choice_doc_fontsize, "default");
fl_addto_choice(class_->choice_doc_fontsize,
tclass.opt_fontsize().c_str());
fl_set_choice_text(class_->choice_doc_fontsize,
params.fontsize.c_str());
fl_clear_choice(class_->choice_doc_pagestyle);
fl_addto_choice(class_->choice_doc_pagestyle, "default");
fl_addto_choice(class_->choice_doc_pagestyle,
tclass.opt_pagestyle().c_str());
fl_set_choice_text(class_->choice_doc_pagestyle,
params.pagestyle.c_str());
}
void FormDocument::class_update(BufferParams const & params)
{
if (!class_.get())
return;
UpdateClassParams(params);
fl_set_choice_text(class_->choice_doc_fonts,
params.fonts.c_str());
fl_set_choice_text(class_->choice_doc_fontsize, //NOT necessary IMHO
params.fontsize.c_str());
fl_set_choice_text(class_->choice_doc_pagestyle, //NOT necessary IMHO
params.pagestyle.c_str());
fl_set_button(class_->radio_doc_indent, 0);
fl_set_button(class_->radio_doc_skip, 0);
[. . .]
