Feedback before proposing a TIP...

[Georgios Petasis]

Hi all,

I am missing some functionality regarding tags from the text widget, and
I would like to propose a TIP about this. Any comments that will
increase the possibility of such a TIP being accepted, are welcomed.
The draft of the TIP is as follows:



Abstract

The canvas widget has a C API to extend the types of canvas items. This
TIP is
an attempt to provide custom tag types for the text widget.

Rationale

The text widget tag support is nice, but limited. For example, with the
existing implementation, the following are not supported:

a) Drawing a tag that is not a rectangular box. For example, you cannot
draw a
wavy underline, a standard way to mark spelling or syntax errors in editors.

b) Overlapping tags that are visually distinguishable. If two different tags
cover the same span, it is impossible to see this in the widget, as only one
is drawn (the one that is higher in the stacking order). There is no way to
"mix" the two tags, so as both are visible (i.e. make the height of the
lower tag greater, so as to appear as two stripes of different colour at
the top or bottom of the tag).

Proposal

Provide two new public functions: Tk_CreateTextTagType, Tk_GetTextTagTypes
(similar to the existing ones for canvas, Tk_CreateItemType,
Tk_GetItemTypes).

Data Structure

The first data structure is Tk_TextTagType (similar to Tk_ItemType):

typedef struct Tk_TextTagType {
const char *name;
int itemSize;
Tk_TextTagCreateProc *createProc;
const Tk_ConfigSpec *configSpecs;
Tk_TextTagConfigureProc *configProc;
Tk_TextTagDeleteProc *deleteProc;

Tk_TextTagDisplayBackgroundProc *displayBackgroundProc;
Tk_TextTagDisplay3DVerticalBevelProc *display3DVerticalBevelProc;
Tk_TextTagDisplay3DHorizontalBevelProc *display3DHorizontalBevelProc;
Tk_TextTagType *nextPtr;
} Tk_TextTagType;

A C extension can provide an additional type, by filling the above
structure,
and registering it with Tk_CreateTextTagType().

The text tags configuration options are extended with a new option, -type.
This can be used by the caller to specify a tag type (which can be changed
with tag configure).

In addition, the following configuration tag options are proposed:

-ipadl(eft): how much internal padding to leave to the left of the tag
-ipadr(ight): how much internal padding to leave to the right of the tag
-ipadt(op): how much internal padding to leave to the top of the tag
-ipadb(ottom): how much internal padding to leave to the bottom of the tag

-opadl(eft): how much internal padding to leave to the left of the tag,
measured from the border of another tag spanning the first character of the
tag.
-opadr(ight): how much internal padding to leave to the right of the tag,
measured from the border of another tag spanning the last character of the
tag.
-opadt(op): how much internal padding to leave to the top of the tag,
measured from the border of another tag spanning any character of the
tag.
-opadb(ottom): how much internal padding to leave to the bottom of the tag,
measured from the border of another tag spanning any character of the
tag.

Discussion

The tag background is drawn from Tk internal function
DisplayLineBackground().
Currently, this function calls the following functions to draw the tag
background:

XFillRectangle()
Tk_3DVerticalBevel()
Tk_3DHorizontalBevel()

Instead of these functions, Tk will have to call the corresponding functions
from the tag's type structure:

XFillRectangle() -> displayBackgroundProc()
Tk_3DVerticalBevel() -> display3DVerticalBevelProc()
Tk_3DHorizontalBevel() -> display3DHorizontalBevelProc()

The last two functions will have an additional flag, denoting whether it is
the top/ down or left/right bevel (in cases it needs to be drawn
differently,
as in wavy underline for spell checking).

Regarding the case of drawing a wavy line under a tag, the
displayBackgroundProc() may just call XFillRectangle() with the
background colour of the widget, the display3DVerticalBevelProc() may
draw nothing, and display3DHorizontalBevelProc() may draw the wavy line
only when drawing the bottom part of the tag, using the value of -ipadb
for the wavy line height (and having a borderwidth of 0). And with a
-opadb != 0 value, the wavy line can be drawn below another (raised
above)tag.

Regarding the case of overlapping tags, these can be drawn extended,
using the
padding specified in the four -opad* configuration options, if they are
overlapped by another tag.

George

[arjenmarkus]

On 2011-12-26 23:46, Georgios Petasis wrote:
> Hi all,
>
> I am missing some functionality regarding tags from the text widget, and
> I would like to propose a TIP about this. Any comments that will
> increase the possibility of such a TIP being accepted, are welcomed.
> The draft of the TIP is as follows:
>
>

Hi George,

your proposal looks quite intriguing, though I have not fathomed all
details yet. It did make me think of some other features I have seen
with text editors (notably of the program source code variety):
- Hide or show portions of the text. This can now be done with the
-elide option of course, but adding a small icon to show text was
hidden would be nice. (I think it can already be done, but I have not
tried it. Possibly an extension for Tlkib's ctext?)
- Show vertical lines to highlight the structure of the source code.
This is definitely impossible at the moment. I could imagine a
tag option that sets the colour of such a line and another one that
sets the positions of where to draw the lines.
They could be used to display tables too.

Well, just some wild ideas.

Regards,

Arjen

[Georgios Petasis]

I think display wise these can be easy:

I.e. add a tag to all the lines a function has, of a type that can draw
a bracket and a small +/- symbol on the left (with the colour, width &
whether it's open or not, customisable through additional configuration
options the tag type has).

But adding the bindings to do all these, is much trickier (i.e. identify
that the symbol was clicked, and not any other part of the tag that
spans several lines.

George

[Francois Vogel]

arjenmarkus said on 27/12/2011 11:53:

> - Hide or show portions of the text. This can now be done with the
> -elide option of course

Well, not so obviously.

Tk bug 3021557 really is a showstopper (for me at least):

https://sourceforge.net/tracker/?func=detail&aid=3021557&group_id=12997&atid=112997

Francois

[Francois Vogel]

Georgios Petasis said on 26/12/2011 23:46:

> I am missing some functionality regarding tags from the text widget,
> and I would like to propose a TIP about this.

Me too, for instance:

https://sourceforge.net/tracker/?func=detail&aid=1759972&group_id=12997&atid=362997

http://groups.google.com/group/comp.lang.tcl/browse_frm/thread/d60973524b786c0e/14a82aef3b1dd24f

I'm wondering if a broader proposal encompassing your draft TIP and a
few further improvements such as the above could be worked out with a
chance to be accepted.

Francois

[Alexandre Ferrieux]

On Dec 27, 8:24 pm, Francois Vogel <fsvogelnew5NOS...@free.fr> wrote:
> Georgios Petasis said on 26/12/2011 23:46:
>
> > I am missing some functionality regarding tags from the text widget,
> > and I would like to propose a TIP about this.
>
> Me too, for instance:
>

> https://sourceforge.net/tracker/?func=detail&aid=1759972&group_id=129...
>
> http://groups.google.com/group/comp.lang.tcl/browse_frm/thread/d60973...

>
> I'm wondering if a broader proposal encompassing your draft TIP and a
> few further improvements such as the above could be worked out with a
> chance to be accepted.

Providing an implementation would certainly tip the balance. When
there is no easy or efficient script-level emulation and no long-range
ill effects on code maintainability (which seems to be the case here),
the problem is more to find an implementor than to fight objections.

-Alex

[Georgios Petasis]

Its always the same awkward situation when I submit TIPs. I don't want
to spend time in implementing it until I have feedback when the TIP has
changes or not, and the feedback never seems to happen without a patch :-)

Maybe a hint on submitted TIPs would be nice (i.e. the one with -time
options for selection, the extended mouse buttons). I suspect that
implementators would be easier to find if we have an idea what we want
in the core, and what we don't.

George

[Donal K. Fellows]

On 28/12/2011 08:16, Georgios Petasis wrote:
> Its always the same awkward situation when I submit TIPs. I don't want
> to spend time in implementing it until I have feedback when the TIP has
> changes or not, and the feedback never seems to happen without a patch :-)

Nothing gets done unless there's a patch or a _very_ strong commitment
to produce one (i.e., an actual promise of time/resources). But now that
we've got everything in Fossil, we can really be stricter about having
the patch before the vote, as we can have it in a feature branch without
causing problems.

> Maybe a hint on submitted TIPs would be nice (i.e. the one with -time
> options for selection, the extended mouse buttons). I suspect that
> implementators would be easier to find if we have an idea what we want
> in the core, and what we don't.

I think the one with time handling in events is reasonable (if highly
technical; people shouldn't be messing with event times unless they know
what they're doing, of course). The ones with mouse handling are more
worrying because of the issue of the limited number of mask bits
available for handling things like dragging; just handling clicks would
be fine. (I don't think anyone is actually objecting in principle to
supporting mice with many buttons; just trying to understand some of the
more subtle consequences.)

Donal.

[Joe English]

Georgios Petasis wrote:
>
> I am missing some functionality regarding tags from the text widget, and

> I would like to propose a TIP about this. [...]

A couple suggestions:

> In addition, the following configuration tag options are proposed:
>
> -ipadl(eft): how much internal padding to leave to the left of the tag
> -ipadr(ight): how much internal padding to leave to the right of the tag
> -ipadt(op): how much internal padding to leave to the top of the tag
> -ipadb(ottom): how much internal padding to leave to the bottom of the tag

I'd suggest folding these into a single "-padding" option as per
Ttk_GetPaddingFromObj(). (That is: a list of up to four elements
specifying the left, top, right, and bottom padding; right and bottom
default to left and top; top defaults to left.) That makes it convenient
to specify uniform padding (one element), separate horizontal and vertical
padding (two elements), or completely asymmetric padding (four elements).

Similarly for -opad{l(eft)/r(ight)/t(op)/b(ottom)}; suggest
calling this "-margins" (that's what Tile and CSS call it).

> The first data structure is Tk_TextTagType (similar to Tk_ItemType):
>
> typedef struct Tk_TextTagType {
> const char *name;
> int itemSize;
> Tk_TextTagCreateProc *createProc;
> const Tk_ConfigSpec *configSpecs;
> Tk_TextTagConfigureProc *configProc;
> Tk_TextTagDeleteProc *deleteProc;
>
> Tk_TextTagDisplayBackgroundProc *displayBackgroundProc;
> Tk_TextTagDisplay3DVerticalBevelProc *display3DVerticalBevelProc;
> Tk_TextTagDisplay3DHorizontalBevelProc *display3DHorizontalBevelProc;
> Tk_TextTagType *nextPtr;
> } Tk_TextTagType;

What is the proposed signature of Tk_TextTagDisplayXXXProc()?

What is the meaning of the 'itemSize' member?

I'd suggest replacing the three displayXXXProc callbacks
with a single displayProc. The motivating use case for
this proposal (spell-check undersquiggles) doesn't even
have 3D bevels, so the callbacks are misleadingly named
at best, and most likely unnecessary.

Do you have a rough idea of how feasible this would be
to implement? (I haven't looked; the text widget internals
have gotten really scary.)


--Joe English

[Georgios Petasis]

Στις 31/12/2011 02:33, ο/η Joe English έγραψε:
> Georgios Petasis wrote:
>>
>> I am missing some functionality regarding tags from the text widget, and
>> I would like to propose a TIP about this. [...]
>
> A couple suggestions:
>
>> In addition, the following configuration tag options are proposed:
>>
>> -ipadl(eft): how much internal padding to leave to the left of the tag
>> -ipadr(ight): how much internal padding to leave to the right of the tag
>> -ipadt(op): how much internal padding to leave to the top of the tag
>> -ipadb(ottom): how much internal padding to leave to the bottom of the tag
>
> I'd suggest folding these into a single "-padding" option as per
> Ttk_GetPaddingFromObj(). (That is: a list of up to four elements
> specifying the left, top, right, and bottom padding; right and bottom
> default to left and top; top defaults to left.) That makes it convenient
> to specify uniform padding (one element), separate horizontal and vertical
> padding (two elements), or completely asymmetric padding (four elements).

This is a good proposal, and Ttk_GetPaddingFromObj is now in the core...

>
> Similarly for -opad{l(eft)/r(ight)/t(op)/b(ottom)}; suggest
> calling this "-margins" (that's what Tile and CSS call it).

Also a good idea.

(To say the truth it cross my mind what would be required to draw tags
using the Ttk engine, but I have postponed it at a latter stage).

>
>> The first data structure is Tk_TextTagType (similar to Tk_ItemType):
>>
>> typedef struct Tk_TextTagType {
>> const char *name;
>> int itemSize;
>> Tk_TextTagCreateProc *createProc;
>> const Tk_ConfigSpec *configSpecs;
>> Tk_TextTagConfigureProc *configProc;
>> Tk_TextTagDeleteProc *deleteProc;
>>
>> Tk_TextTagDisplayBackgroundProc *displayBackgroundProc;
>> Tk_TextTagDisplay3DVerticalBevelProc *display3DVerticalBevelProc;
>> Tk_TextTagDisplay3DHorizontalBevelProc *display3DHorizontalBevelProc;
>> Tk_TextTagType *nextPtr;
>> } Tk_TextTagType;
>
>
> What is the proposed signature of Tk_TextTagDisplayXXXProc()?
>
> What is the meaning of the 'itemSize' member?

It is an idea taken from the canvas implementation. It allows the
specification of a larger structure, whose first element is the
Tk_TextTagType structure, like the BitmapItem example:

http://docs.activestate.com/activetcl/8.6/tcl/TkLib/CrtItemType.htm

>
> I'd suggest replacing the three displayXXXProc callbacks
> with a single displayProc. The motivating use case for
> this proposal (spell-check undersquiggles) doesn't even
> have 3D bevels, so the callbacks are misleadingly named
> at best, and most likely unnecessary.

This was my initial thought (all drawing by a single routine),
but I gave up on the idea because of performance considerations.
The text widget draws areas of the same tag characteristics as a uniform
area, with borders drawn only at the edges.
Moving all this logic to the drawing routine, may not be a good idea.
But I am not so sure about all these, of course.

>
> Do you have a rough idea of how feasible this would be
> to implement? (I haven't looked; the text widget internals
> have gotten really scary.)

Yes, it is going to be quite difficult. I have tried to implement the
-ipad options (not that difficult). But the real problem is that the
current code examines all tags for an index, and merges them into a
single visual presentation, based on their priority.

This has to change :-)

George

[Francois Vogel]

Alexandre Ferrieux said on 28/12/2011 01:07:

> Providing an implementation would certainly tip the balance. When
> there is no easy or efficient script-level emulation and no long-range
> ill effects on code maintainability (which seems to be the case here),
> the problem is more to find an implementor than to fight objections.

OK, I have tried to take up the challenge.
I'm now having a tentative patch proposing an implementation for Tk
Feature Requests item #1759972 (well, part of it, as a first step)
against the latest fossil sources.

What is the recommended method to expose the patch for peer review
(since I can't attach a file to item #1759972)?

Francois

[Alexandre Ferrieux]

On Jan 5, 8:09 am, Francois Vogel <fsvogelnew5NOS...@free.fr> wrote:
> Alexandre Ferrieux said on 28/12/2011 01:07:
>
> > Providing an implementation would certainly tip the balance. When
> > there is no easy or efficient script-level emulation and no long-range
> > ill effects on code maintainability (which seems to be the case here),
> > the problem is more to find an implementor than to fight objections.
>
> OK, I have tried to take up the challenge.
> I'm now having a tentative patch proposing an implementation for Tk
> Feature Requests item #1759972 (well, part of it, as a first step)
> against the latest fossil sources.

Sorry, SF does not seem to have an artifact by that number in Tk.
Pasting an URL avoids this kind of typo ;-)

>
> What is the recommended method to expose the patch for peer review
> (since I can't attach a file to item #1759972)?

That's an unnerving design mistake about the "Feature Requests" and
"Patches" trackers: the request naturally begins its life in the
former, but the much needed attachment capability only belongs to the
latter. Bummer. Just create a new patch request, and add a comment
pointing to it in the feature request.

-Alex

[Francois Vogel]

On 5 jan, 09:40, Alexandre Ferrieux <alexandre.ferri...@gmail.com>
wrote:

> > I'm now having a tentative patch proposing an implementation for Tk
> > Feature Requests item #1759972 (well, part of it, as a first step)
> > against the latest fossil sources.
>
> Sorry, SF does not seem to have an artifact by that number in Tk.
> Pasting an URL avoids this kind of typo ;-)

Uh?
This is not a typo. I copy/pasted from the message sent by SF when
monitoring this artifact.

Put this in Google: Tk Feature Requests item #1759972
and you'll get it as the first match.

> create a new patch request, and add a comment
> pointing to it in the feature request.

OK, I will do it.

Francois

[Georgios Petasis]

I searched the feature requests with the id provided, and I found this:

http://sourceforge.net/tracker/?func=detail&aid=1759972&group_id=12997&atid=362997

Is this the feature request the patch is for?

George

[Francois Vogel]

Georgios Petasis said on 05/01/2012 12:30:

>
> I searched the feature requests with the id provided, and I found this:
>
> http://sourceforge.net/tracker/?func=detail&aid=1759972&group_id=12997&atid=362997
>
> Is this the feature request the patch is for?

Yes it is.

Francois

[Francois Vogel]

>> create a new patch request, and add a comment
>> pointing to it in the feature request.

Done.

https://sourceforge.net/tracker/?func=detail&aid=3469780&group_id=12997&atid=312997

Francois

[Alexandre Ferrieux]

On Jan 5, 11:30 am, Francois Vogel <fsvogeln...@free.fr> wrote:
> On 5 jan, 09:40, Alexandre Ferrieux <alexandre.ferri...@gmail.com>
> wrote:
>
> > > I'm now having a tentative patch proposing an implementation for Tk
> > > Feature Requests item #1759972 (well, part of it, as a first step)
> > > against the latest fossil sources.
>
> > Sorry, SF does not seem to have an artifact by that number in Tk.
> > Pasting an URL avoids this kind of typo ;-)
>
> Uh?
> This is not a typo. I copy/pasted from the message sent by SF when
> monitoring this artifact.

Ooops, turns out the global search in SF no longer indexes artifact
IDs, as it formerly did:

https://sourceforge.net/search/?group_artifact_id=362997&type_of_search=artifact&group_id=12997&words=1759972

Sorry for accusing you :}

-Alex