[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ktexteditor-devel
Subject:    Re: [RFC] new Interface to provide custom label/line
From:       Andreas Pakulat <apaku () gmx ! de>
Date:       2008-03-23 22:58:52
Message-ID: 20080323225843.GB10993 () morpheus ! apaku ! dnsalias ! org
[Download RAW message or body]

On 23.03.08 15:09:09, Dominik Haumann wrote:
> On Saturday 22 March 2008, Andreas Pakulat wrote:
> > On 02.03.08 14:21:22, Andreas Pakulat wrote:
> > Lets see if providing patches gets some more responses. So attached is a
> > diff that adds a couple of interface classes to KTE. Namely thats:
> > 
> > LineAnnotationInterface - a document interface that simply allows to
> > associate a LineAnnotationProvider with a document. As the annotation
> > information normally isn't different for different views of a document I
> > think this is the right place to hook this up
> > 
> > LineAnnotationProvider - a simple remotely-model-like interface to
> > retrieve the needed information for an annotation. Its simply a short
> > text, a longer text and a way to influence the visual style.
> > 
> > AnnotationInteractionInterface - this is a view interface that allows to
> > do interactions with the annotation border, like executing code when an
> > annotation is clicked on, or providing a context menu. (In kdevelop's
> > vcs suppor this could be used to show the diff for a given revision or
> > to display more verbose information in the long text).
> > 
> > My next step is to find the code that produces the line-number-border
> > and try to see wether I can use it as base for an implementation of the
> > above.
> > 
> > Question? Comments?
> > 
> > Andreas
> > 
> > PS: If anybody has better naming ideas I'm all ears. The above is just
> > what my limited mind could come up with :)
> 
> 1. Implementations
> Just like with other classes who have very small .cpp files (i.e. only virtual
> destructors) put the implementations into ktexteditor.cpp.

Ok.

> 2. AnnotationInteractionInterface (View extension interface)
> 
> a) Right now, if an annotation provider is set, all views show the annotation
> border. However, line numbers etc are configurable for every view
> independently. This is inconsistent. Possble solution: Add a
> showAnnotations(bool enable) to AnnotationInteractionInterface.

Well, I wasn't sure wether that should be exposed or not (actually I
haven't checked wether thats possible for the line-number border).

> + virtual void annotationContextMenuAboutToShow( View* view, QMenu* menu, int line \
> ) = 0; 
> *think* Isn't that already enough?
> 
> Kate Part impl could be something like:
> void KateView::contextMenuEvent()
> {
> QMenu menu;
> // let the annotation clients can fill the menu... they can connect to
> // the QAction immediately and get what happens this way
> emit annotationContextMenuAboutToShow(this, &menu, the_line);
> if (!menu.isEmpty()) {
> // aha, someone added items -> show the menu
> }
> }
> 
> I don't see why those are needed in addition. Do you have a use case? :)
> +    virtual void setAnnotationContextMenu( QMenu* menu ) = 0;
> +    virtual QMenu* annotationContextMenu( QMenu* ) const = 0;
> +    virtual QMenu* defaultAnnotationContextMenu( QMenu* menu = 0 ) const = 0;

I just followed the style for KTE::View here, the proposed API would
allow to have a "hide annotation border" item in
defaultAnnotationContextMenu and maybe other things too...

> I can imagine that when clicked you can choose a revision for diffing.
> +    virtual void annotationLineClicked( View* view, int line ) = 0;
> 
> What's the use case for double clicking? Especially because a double click
> always emits a single click in advance.
> +    virtual void annotationLineDoubleClicked( View* view, int line ) = 0;
> If we don't need this, we could even think about renaming annotationLineClicked
> to annotationLineActivated.

Yeah and it should probably use the KDE wide setting for activation
(i.e. double or single click). 
 
> b) svn annotate is of the form
> <revision>    <author>    <text>
> ShortAnnotationRole is revision + author in this case, isn't it?

I actually thought about using only one of the two for the short role so
the border doesn't get too wide.

Also the full annotation will show revision, author, commit-msg and date

> svn annotate aligns the information in columns. So I think it would make
> sense to separate this, too. If ShortAnnotationRole is revision + author
> then we cannot have the nice alignment, or do I miss something?

Right, if you think its important to see both we probably need to have
separate roles (which would somehow bind the interface a bit to VCS
systems) or return a list of texts (or a custom datatype).

> PS: Personally, I find the revision number very important, you can e.g.
> discard an old commit immediately, or see it's the same as others lines.

/me too.

> *Thinking about use cases*
> First is of course a VCS. there you have revision + author + line.
> Otoh, it could also be collaborative editing, where we maybe only want to
> set a name (e.g. line changed by userX).
> ...which leads to the question: Is this meant for VCS only?

No, I also thought about collaborative editing to see the author of a
given line.

> c) Can we reuse Qt::ItemDataRole here instead of defining a new one?

We could.

> At least we can map the existing items pretty good:
> ShortAnnotationRole -> DisplayRole
> FullAnnotationRole  -> ToolTipRole
> Backgroundrole      -> Backgroundrole
> ForegroundRole      -> ForegroundRole

Those make total sense to me.

> <missing>           -> Qt::UserRole or Qt::EditRole (== revision)

I think I'd go for user-role here, so the annotation provider can
provide an additional column of information if he wants to. For VCS this
would be the revision, for other use-cases there might be other
information here (like date for collaborative editing)

> Of course the documentation still has to state what roles are used
> for what purposes. Otoh, maybe this simply doesn't make sense and we simply
> should use a new enum.

No, I'm fine with using Qt::ItemDataRole.

> 4. LineAnnotationInterface
> Right now it's a Document extension interface. If we decide to make it a
> view extension interface only we could remove this interface completely
> and move the two functions
> +    virtual voie setAnnotationProvider( AnnotationProvider* provider ) = 0;
> +    virtual AnnotationProvider* annotationProvider() const = 0;
> to the View.

Yes, but as I said the annotation is IMHO something that belongs to the
document, i.e. I don't see two views having different annotation
providers. It would also allow to automatically have the annotation
visible on a new view if the user wants that.

> 6. Documentation
> Typo, if we stick to LineAnnotationInterface :)
> -Q_DECLARE_INTERFACE(KTextEditor::AnnotationInterface, \
> "org.kde.KTextEditor.AnnotationInterface") \
> +Q_DECLARE_INTERFACE(KTextEditor::LineAnnotationInterface, \
> "org.kde.KTextEditor.LineAnnotationInterface")

Oh, yeah :)

> enum DataRole:
> +      ShortAnnotationRole /** This identifies a short annotation information,
> +                           * like an author name*/,
> Those doxygen comments do not work, either use ///< or/**<, or if the
> documentation is long, it makes sense to move it before the item:

Ok, haven't tried to build the api docs yet. But I guess that explains
why the enum-docs in KDevelop arent' properly shown :)

> 7. Thoughts
> Do you plan to implement it in Kate Part for KDE 4.1? If not, it makes
> sense to move the integration to KDE 4.2, so we can still change the
> KTE interface.

I'm already on the implementation (though I don't expect to have
something usable before the end of next weekend), because that would
mean kdevelop doesn't have to provide a standalone vcs dialog for its
first release (as kdevelop4.0 will probably depend on KDE 4.1). 

However I'm aware that designing and interface takes time, so if there's
not enough time left I'm not going to insist on leaving something
incomplete in kdelibs.

Andreas

-- 
"Life, loathe it or ignore it, you can't like it."
		-- Marvin, "Hitchhiker's Guide to the Galaxy"
_______________________________________________
KTextEditor-Devel mailing list
KTextEditor-Devel@kde.org
https://mail.kde.org/mailman/listinfo/ktexteditor-devel


[prev in list] [next in list] [prev in thread] [next in thread]