Re: Default settings for GtkTextAttributes

[Brian Cameron <Brian Cameron Sun COM>]

Havoc:

Havoc Pennington <hp redhat com> writes:
> Brian Cameron <Brian Cameron Sun COM> writes:
> > Therefore it seems like we could save ourselves a *lot* of work by
> > not re-inventing the wheel. For our own purposes we need to know, for
> > most attributes, whether they were explicitly set (via tags) or simply
> > "inherited" from the default.
> 
> This is to read text properties to blind users, etc., right?

Exactly.  Although I'm not sure that the interface is purely limited
to blind and visually impaired persons.

> I would guess that what you actually want is simply:
> 
>  GtkTextAttributes *default_attrs;
>  GtkTextAttributes *actual_attrs;
> 
>  default_attrs = gtk_text_view_get_default_attributes (textview);
>  actual_attrs = gtk_text_attributes_copy (default_attrs);
>  if (gtk_text_iter_get_attributes (&iter, actual_attrs))
>    {
>       /* Compare default_attrs to actual_attrs */
> 
>       if (default_attrs->size != actual_attrs->size)
>          /* report different size */    ;
>  
>       /* etc, for each field in attrs */
>    }
> 
> I don't see why you'd want to report differences that aren't visible
> to sighted users. (i.e. if a sighted user can't tell that the text
> properties are changed from the default, why would a blind user want
> to know either? this is just some program implementation detail.)

Unfortunately, there are situations where a tag can cause a change that
is invisible to sighted users, but the information would need to be made
available through the ATK

A good example would be a program like Emacspeak, a program that is
very context sensitive.  For example.  If you are looking at a C
program in Emacspeak it might show functions in blue, comments in
green, arguments in white, etc.  Emacspeak is smart enough to
communicate this to a blind person.  So, Emacspeak reads the
text and also the context information.  So, if it is reading green
text, it knows that this is a comment and says "This is a comment"
before reading the text.

A sighted person might set the theme text color to green and loose
this context information.  However, the theme settings shouldn't 
cause the blind person to loose this information.  Especially since
a blind person shouldn't have to care what theme their desktop is
using.  It would be a bad design if the behavior of programs like
Emacspeak were to change if the user switches between different
desktops that have different theme settings.

Therefore, if we can can know what settings are set via tags as
opposed to via default settings, then we can avoid such problems.

> >    We will also need a public accessor for
> >    view->layout->default_style
> 
> We've agreed that we need gtk_text_view_get_default_attributes().

Excellent.  Thanks much!

> > Again I'm a little confused since it doesn't seem that the
> > gtk_text_iter_get_attributes() function returns any default values or
> > accesses view->layout->default_style in any way.  It seems that it
> > depends on the gtk_text_attributes_new() function to fill in the 
> > default values before calling gtk_text_iter_get_attributes().  Is
> > this understanding incorrect?
> 
> Yes, this is wrong. You would normally get the attributes to pass in
> by:
>   a) calling gtk_text_attributes_new ()
>   b) filling in some sane default values
> 
> so you are skipping step b). When GtkTextView uses the
> get_attributes() function it does not skip step b), it computes
> layout->default_style.

Thanks much for explaining this to me.  It's all becoming clear.  :)

> What you're doing is trying to get the values that GtkTextView would
> compute, this is a view-specific operation. So it should involve a
> function which is view-specific, such as
> gtk_text_view_get_default_attributes().
> 
> The reason gtk_text_attributes_new() doesn't fill in better default
> values is that there really aren't any that make a lot of sense, so
> why waste the CPU cycles. Defaults need to come from the view.
> gtk_text_attributes_new() simply allocates a new "blank"
> GtkTextAttributes. Think of it as equivalent to the declaration "int i;".
> 
> > As a separate issue, we still think it is a bug that the 
> > gtk_text_attributes_new() function is broken.  It doesn't seem to be
> > setting the values to the defaults from view->layout->default_style.
> 
> How could it do that? The text buffer and text attributes know nothing
> about the view. There can be multiple GtkTextView viewing a single
> GtkTextBuffer. Using view->layout->default_style would screw up the
> model-view separation.

Thanks for reminding me of this.  Yes, you are right that since a buffer
can have multiple views, that gtk_text_attributes_new() cannot set the
values to the defaults from view->layout->default style.  

However, there is still no reason why each value in the GtkTextAttributes
structure can have a setting which means "unset" which the
gtk_text_attributes_new() sets them to.  In our situation this would be
very useful, and in your situation, the values would get reset to the
default values.

Clearly a problem with doing this is that changing the GtkTextAttributes
structure so drastically is fairly intrusive.  Especially to do it properly.
All enumerations will need an "UNSET" value and all booleans would have
to be changed to enumerations with TRUE, FALSE, and "UNSET" values, and
possibly other things would have to change as well.

I have a proposal for a less intrusive change that would meet everybody's
needs a little more cleanly.

I notice that the function gtk_text_iter_get_attributes calls 
_gtk_text_attributes_fill_from_tags, which actually does the work of
setting the GtkTextAttributes structure (and is the only function that
actually modifies the GtkTextAttributes structure.  The 
_gtk_text_attributes_fill_from_tags() function has a while loop where
it loops over each tag and sets the structure. 

Would it be possible to change the design very slightly so that you
call the gtk_text_iter_get_attributes() like this:

   gtk_text_iter_get_attributes(const GtkTextIter *iter,
      GtkAttributeFunc attribute_func, void * userdata);
      
The additional "attribute_func" parameter would be a function pointer
that would take the following arguments.  The "userdata" value would
take a structure that would be passed into the callback.

   attributes_cb(GtkTextTag *tag, void *userdata); 

Then the _gtk_text_attributes_fill_from_tags function could call this
function in the while loop.  In your situation, you would define 
"userdata" as the GtkTextAttributes structure and your specified function
would fill it (much like the current _gtk_text_attributes_fill_from_tags()
does)  In our situation, we could pass in our structure to fill and write
a function that does exactly what we need.  This would give us the
ability to know *exactly* which tags are set in a straightforward and
clean manner.

A slightly different idea would be to make the callback receive the
array of tags and the callback would loop over them rather than having
the callback be called for each tag separately.  In this situation the
callback would replace the _gtk_text_attributes_fill_from_tags() function.
and the callback would take the same arguments as 
_gtk_text_attributes_fill_from_tags().

   attributes_cb(GtkTextTag ** tags, guint n_tags, void * userdata);

This might be a little more efficient.

How does this proposal sound?  We are happy to do this work and supply
a patch if it will help this change be made.

Brian