Abdelrazak Younes <[EMAIL PROTECTED]> writes:
>
> Angus Leeming wrote:
> > Abdelrazak Younes <younes.a <at> ...> writes:
> >> Grrrr. gmane's web interface is a PITA!
>
> Why don't you use
It's a personal thing (and probably ridiculous ;-)) I'm
happy reading a web page on a work machine; less happy installing
software to enable me to do so.
> > Here're my comments about the code. Executive summary:
> > I don't like the fact that Application and WorkaArea
> > both derive from Clipboard.
> Of Application? I agree but I moved the code carefully
> so that I don't break everything at the same time.
That's fine then. If the issue comes down to trust,
then I'm perfectly happy to trust you.
> > Would user code not be cleaner if the Clipboard class
> >defined the getter/setter as
> >
> > std::string operator()() const;
> > void operator()(std::string const &);
> >
> > ? Maybe not
>
> Could you elaborate please?
std::string clipcontents =
some_container.clipboard().getClipboard();
seems so redundant. If you introduced Clipboard::operator() to
get/set its contents, then user code would become
std::string clipcontents =
some_container.clipboard()();
but maybe that's too short. Alternatively, you might name
these member functions as
class Clipboard {
public:
std::string const & contents() const;
void contents(std::string const &);
};
>
> >> Index: src/BufferView_pimpl.C
> >> ===================================================================
> >> <at> <at> -145,6 +146,12 <at> <at>
> >> workarea_.reset(WorkAreaFactory::create(*owner_, width, height));
> >> screen_.reset(LyXScreenFactory::create(workarea()));
> >>
> >> + // FIXME (Abdel 04/06/2006)
> >> + // For now, clipboard_ contains the same object as workarea_
> >> + // This cannot be changed because of xforms.
> >> + // A static_cast is not possible here...
> >> + clipboard_.reset((Clipboard *) (workarea_.get()));
> >
> > <shrug>So fix the xforms code too</shrug>
>
> The attached patch get rid of this C-ish cast by defining a new
> interface in WorkAreaFactory:
>
> /**
> * Get the toolkit-specific clipboard instance.
> */
> Clipboard * getClipboard();
>
> But ultimately, I think we need to take a decision about xforms.
So, work out why it's so much quicker than the Qt frontend, fix that
and bin xforms ;-)
>
> [...]
> >
> > Noooooooooooooo!!!!!!!!!!!!
> >
> > No anonymous namespaces in .h files please.
> > Will lead to ODR problems.
>
> Don't shout! I didn't know
> What's ODR?
One definition rule. It's one of those areas where C++ gets
really messy. What it comes down to is that the namespace
is given a unique name in the translation unit (.cpp file)
that you're compiling. And, of course, you're going to compile
several differnt .cpp files that include this header, so all
of a sudden you're going to have multiple functions where
you thought you had one.
Nasty, huh?
> OOOOOOOOOOOOOOO KKEEEEEEEEEYYYYYYY !!!!
>
> Note that I didn't say that this was the final proposal.
> Or is it forbidden to post work-in-progress patch?
;-)
Of course not. I was commenting as I was going through the patch.
Or should I write an elegant essay with cross references to your patch?
;-)
Nice work by the way! It's great to see you attacking
ugly code with so much gusto.
Angus
