Re: [Cvslog] r21400 - in /lyx-devel/trunk/src: CutAndPaste.cpp insets/...

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Cvslog] r21400 - in /lyx-devel/trunk/src: CutAndPaste.cpp insets/...

Martin Vermeer-2
On Sat, Nov 03, 2007 at 09:03:09AM +0000, [hidden email] wrote:

> Author: younes
> Date: Sat Nov  3 10:03:08 2007
> New Revision: 21400
>
> URL: http://www.lyx.org/trac/changeset/21400
> Log:
> Transfer some code from InsetFlex to InsetCollapsable. Add some safeguards and FIXMEs.
>
> Modified:
>     lyx-devel/trunk/src/CutAndPaste.cpp
>     lyx-devel/trunk/src/insets/InsetCollapsable.cpp
>     lyx-devel/trunk/src/insets/InsetCollapsable.h
>     lyx-devel/trunk/src/insets/InsetFlex.cpp
>     lyx-devel/trunk/src/insets/InsetFlex.h
>     lyx-devel/trunk/src/tex2lyx/Font.h

...
 
> + setLayout(buf.params());
> +
>   if (!token_found)
>   status_ = isOpen() ? Open : Collapsed;
> -
> - setLayout(buf.params());

Is there a reason for this swap?

...
 

>  
>  InsetCollapsable::Decoration InsetCollapsable::decoration() const
>  {
> - if (layout_->decoration == "classic")
> + if (!layout_ || layout_->decoration == "classic")
>   return Classic;
>   if (layout_->decoration == "minimalistic")
>   return Minimalistic;
>   if (layout_->decoration == "conglomerate")
>   return Conglomerate;
> - if (name() == from_ascii("Flex"))
> + if (lyxCode() == FLEX_CODE)

Why is this better?

> + // FIXME: Is this really necessary?
>   return Conglomerate;

Not if you put "Decoration Conglomerate" in all the charstyle entries in
the layout files.

...

>  int InsetCollapsable::latex(Buffer const & buf, odocstream & os,
>    OutputParams const & runparams) const
>  {
> + // FIXME: What should we do layout_ is 0?
> + // 1) assert
> + // 2) through an error
              throw

> + if (!layout_)
> + return 0;
> +

Suspenders and a belt... should not happen with a well set up layout
file environment (see files stdcharstyles.inc and stdcustom.inc) but
doesn't harm. I would just assert, and describe the fix in a comment
at the assert location.

...

- Martin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Cvslog] r21400 - in /lyx-devel/trunk/src: CutAndPaste.cpp insets/...

Abdelrazak Younes
Martin Vermeer wrote:

> On Sat, Nov 03, 2007 at 09:03:09AM +0000, [hidden email] wrote:
>> Author: younes
>> Date: Sat Nov  3 10:03:08 2007
>> New Revision: 21400
>>
>> URL: http://www.lyx.org/trac/changeset/21400
>> Log:
>> Transfer some code from InsetFlex to InsetCollapsable. Add some safeguards and FIXMEs.
>>
>> Modified:
>>     lyx-devel/trunk/src/CutAndPaste.cpp
>>     lyx-devel/trunk/src/insets/InsetCollapsable.cpp
>>     lyx-devel/trunk/src/insets/InsetCollapsable.h
>>     lyx-devel/trunk/src/insets/InsetFlex.cpp
>>     lyx-devel/trunk/src/insets/InsetFlex.h
>>     lyx-devel/trunk/src/tex2lyx/Font.h
>
> ...
>  
>> + setLayout(buf.params());
>> +
>>   if (!token_found)
>>   status_ = isOpen() ? Open : Collapsed;
>> -
>> - setLayout(buf.params());
>
> Is there a reason for this swap?

Yes, isOpen() uses layout_ (and it was using an uninitialized layout_
previously).

>
> ...
>  
>>  
>>  InsetCollapsable::Decoration InsetCollapsable::decoration() const
>>  {
>> - if (layout_->decoration == "classic")
>> + if (!layout_ || layout_->decoration == "classic")
>>   return Classic;
>>   if (layout_->decoration == "minimalistic")
>>   return Minimalistic;
>>   if (layout_->decoration == "conglomerate")
>>   return Conglomerate;
>> - if (name() == from_ascii("Flex"))
>> + if (lyxCode() == FLEX_CODE)
>
> Why is this better?

Not better but name() is not "Flex" anymore but the layout name.

>
>> + // FIXME: Is this really necessary?
>>   return Conglomerate;
>
> Not if you put "Decoration Conglomerate" in all the charstyle entries in
> the layout files.

OK.

>
> ...
>
>>  int InsetCollapsable::latex(Buffer const & buf, odocstream & os,
>>    OutputParams const & runparams) const
>>  {
>> + // FIXME: What should we do layout_ is 0?
>> + // 1) assert
>> + // 2) through an error
>               throw
>
>> + if (!layout_)
>> + return 0;
>> +
>
> Suspenders and a belt... should not happen with a well set up layout
> file environment (see files stdcharstyles.inc and stdcustom.inc) but
> doesn't harm. I would just assert, and describe the fix in a comment
> at the assert location.

OK.

Thanks,
Abdel.

Loading...