[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] product editor patch

From: Greg Chicares
Subject: Re: [lmi] product editor patch
Date: Fri, 08 Feb 2008 16:50:14 +0000
User-agent: Thunderbird (Windows/20071031)

On 2008-02-08 15:41Z, Vaclav Slavik wrote:
> Greg Chicares wrote:
>> (A) 'make check_concinnity' complains about new '.xpm' files.
>> I can fix that with
>>   sed -e's/static const unsigned char \*/static char const\*/'
>> if there's no objection. If you feel that 'unsigned char' is
>> better,
> It doesn't really matter, both ways are different from what image 
> editors output (or at least GIMP and ImageMagick; I don't know if 
> Evgeniy edited the files manually or if he had some tool that did 
> output his version).

They were edited manually, probably by me. IIRC, the lack of
'const' caused a compiler diagnostic (due to the deprecated
conversion in C++2003 4.2/2) on some *nix platform that inlined
the xpm's contents via '#include'--so we added 'const'. Then we
saw we'd need to re-add it manually after editing any file in
GIMP--so, to make sure we wouldn't forget to do that, we made
'make check_concinnity' test for 'const'.

>> I guess the last one has nothing to do with the first two, and
>> is just a stray marker that never got erased. It would be nice
>> to enhance 'make check_concinnity' to find things like that,
>> maybe by invoking the W3C validator.
> I'm not sure how would W3C validator help. We don't have any schema 
> for XRC format that could be validated and the file is well-formed 
> XML file even with this bug in it, so xmllint cannot detect the 
> problem (in absence of a schema forbidding this). But yes, it would 
> be nice to write XML Schema or Schematron rules for XRC format and 
> validate it.

Oh--yes, I had forgotten that there's no schema for XRC yet.

It would still be nice to use the W3C validator for other files
anyway. I've run it manually for webpages, but automatically
would be better than manually.

>> where some smart pointer that was apparently overkill is now
>> a plain stack object that needs 's/->/./'.
> I uploaded revised patch to Savannah with this and other things 
> (AAAA..., XPM files) fixed.

I didn't mean to create more busywork for either you or me.
I was just noting this one issue because if anyone else (in my
office, for instance) applied the original patch without making
this one later change, they might think there was a problem with
the patch--but there isn't.

Ideally, I'd just commit all the product-editor changes to HEAD
right away, and review them later. That was my initial plan,
because I'm highly confident before even looking at the patch
that it's an improvement, and the product editor that's in HEAD
now is masked from end users anyway. But I couldn't commit it
yet--apparently, not because of anything you did wrong, but
because of something you did right: you made error-checking
stronger, and that exposed some latent lmi defects.

And I don't have time to look into those defects, or to answer
Vadim's posting, or to say any more now, because I'm deeply
buried in some taxation issues. But I did want to say that I'm
not demanding that you maintain the patch as, in effect, a
separate branch, updating it to reflect changes in HEAD. That
would be awful. We need to get this merged as soon as we can.
If you can figure out how to fix the lmi defects discussed in
my previous email, that would be most welcome.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]