Re: Working on a first bug - Have some questions

[Kai Willadsen <kai willadsen gmail com>]

On 7 October 2015 at 06:00, David Rabel <David Rabel noresoft com> wrote:
> Hi there,
>
> I am back and now I have some time to get in touch with the meld source
> code. As promised, I will ask questions whenever something is unclear. ;-)
>
> I want to start with "Bug 633515 - Text ignored by filters should have
> some visual indication" [1]. I spent a few hours by browsing through the
> source code and playing around with it.

Awesome! Could you please just make a note on that page that you're
looking at it so that no one else tries to pick it up at the same
time?

> As you, Kai, suggested, I tried to do it with a gtk.TextTag.
> Unfortunately it only works to make something bold but not to make
> something light. It also works to make the font smaller, but that does
> not look to nice.
> I attached three diffs, maybe you could have a look at them? As I said,
> bold and small works, light does not.

So I've had a bit of a play here, and this *does* work to make
something light... but only if the active font has an appropriate
weight variant. I had assumed that we'd get a synthetically lightened
version or something, but apparently I was wrong. Either way, that
kind of makes font weight not an option, since we can't control
available fonts.

> This was only for testing, so the way I hacked it into the code is
> probably not the cleanest way. As you also mentioned, the nicest would
> be to rely on gtk.SourceView, but I haven't checked that too intensively
> so far. But I had a look at the gtk.SourceStyle class [2], which is
> probably the place to start and it also only supports bold text, no
> light text. So maybe that causes the problem I had.
> Is the font-weight even what you had in mind?

I was originally thinking either font weight or text colour... but I
don't have strong feelings about this for now. I'd go with any option
that works.

> I don't know if it is
> possible to make the font color transparent or something. That would be
> nice. But I really don't know how to handle this, especially not to get
> in conflict with the style schemes.

Sure. Pretty much any of our options *might* conflict with something
applied by a style scheme, so we just have to live with that. The vast
majority of style schemes set no background colours, so that's a
fairly reliable fallback if we want it. I'm also not too worried about
overwriting the style scheme's foreground colours, since what we're
indicating in code is unlikely to be overwriting important syntax
highlighting annotations (i.e., you're probably not writing a regex to
ignore function definitions or language keywords).

So for now, I'd ignore the issue of conflicting with style schemes and
make the drawing something that will definitely work, e.g., a light
grey foreground text colour.

I'm not sure if you were done with it, but that patch doesn't quite do
the right thing when you get multiple ignored regions. I think the
problem is that the _filter_text changes assume that it's only called
with the whole file, but we actually call it whenever we request a
slice from meldbuffer.BufferLines (which is quite a bit). I think
you'll have to change the _filter_text call to take buffer offsets
from the slice call and do offset calculations from there.

cheers,
Kai