Line filters not working

Main development forum.

Line filters not working

Postby matthias1955 » Wed May 27, 2009 5:43 pm

related to bugitem #2796768 Line filters not working if "Match similar lines" enabled

The filter actually is only working correct (marked as trivial) if the full diff is matching to linefiter on left and right view.
The same efect we get on option ignore empty lines.

while creating diffs,
we should first ckeck linefilter and, if option Ignore emptyline is set,
break the diff at that lines and mark that diff as trivial.
matthias1955
 
Posts: 162
Joined: Wed Dec 17, 2008 1:55 pm

Re: Line filters not working

Postby kimmov » Thu May 28, 2009 5:50 pm

matthias1955 wrote:The filter actually is only working correct (marked as trivial) if the full diff is matching to linefiter on left and right view.
The same efect we get on option ignore empty lines.

Hmm. I might have understood the report wrong. But I have to check more carefully.

You should remember that ignoring empty lines is still diffutils feature, not filtering feature. So there is nothing you can compare in these features. Totally different code.

We could move empty line igroring to line filtering. But it is not worth doing unless we intend to expand it, i.e. improve it by allowing more configuration to what to ignore.

matthias1955 wrote:while creating diffs,
we should first ckeck linefilter and, if option Ignore emptyline is set,
break the diff at that lines and mark that diff as trivial.

Like I wrote above, this is totally new feature and its not worth doing if there is no some real advantage of it. A lot more important would be to combine comment filtering and line filtering to something sane. Now they duplicate lots of code.
kimmov
 
Posts: 562
Joined: Thu Sep 11, 2008 8:51 pm
Location: Finland

Re: Line filters not working

Postby petervanwesten » Wed Jun 03, 2009 7:46 am

Maybe this helps, or maybe it is stating the obvious.
But it took me quite some time to figure out:

Line filters only work when using File Compare Method 'Full Contents'.
And does not work on 'Quick Contents'.

When using Quick Contents, the files are seen as different, but when you open them, you see it marks the filtered lines correctly (and can't copy side to side because they are 'equal').

So it might be a good idea to notify the user on the fact tat line filters are not used on other methods than Full Contents.
I see that this is explained in the help file, but - like me - a lot of people might not look there on the sight of first problems.
petervanwesten
 
Posts: 4
Joined: Wed Jun 03, 2009 7:24 am

Re: Line filters not working

Postby kimmov » Wed Jun 03, 2009 12:20 pm

There is already bug report about this: #2219579 Doesn't warn that line filters aren't applied.

This one of those cases where it is hard to think about good GUI. Or rather good compromise for GUI. Dialogs and messageboxes become annoying after couple of times. Statusbar texts are not easy to spot. So perhaps this should be handled in restructuring compare method selection GUI so that it tells what every method does.

Some could think that the solution is to enable line filters for quick compare. But the whole point of that quick compare is it does compare as fast as possible. And adding filters would make it slower (though a bit faster than full contents compare). As it is quick compare does just byte per byte compare of file contents. There is no "lines" or other things that full contents handles...
kimmov
 
Posts: 562
Joined: Thu Sep 11, 2008 8:51 pm
Location: Finland

Re: Line filters not working

Postby matthias1955 » Wed Jun 03, 2009 12:31 pm

kimmov wrote:
You should remember that ignoring empty lines is still diffutils feature, not filtering feature.


I got the same effect only.
So if I have an empty line at start or end of a diff, this line is not marked as trivial.
But specially in this point, what is the difference in diffutils and filtering ? I think same.
A filtered line is a ignored line...

Is it a must to have a filtered line on both side?
We can have a filtered line left and a ghostline right, isn't it?.

I created some code to splitt the diffblock. It's working fine so far.
The GUI doesn't change...

petervanwesten wrote a lot of people might not look there on the sight of first problems

user must read the doc once to note what adanced are there. :?
matthias1955
 
Posts: 162
Joined: Wed Dec 17, 2008 1:55 pm

Re: Line filters not working

Postby kimmov » Wed Jun 03, 2009 4:33 pm

Ignoring empty lines ignores only differences that consists only empty lines. So in practice it ignores differences that add/remove empty lines.

This is important option since it is one option to command line diff and we must maintain compatibility to diff. So we can't change how this option works. But we can implement some other feature which ignores empty lines smarter ways.
kimmov
 
Posts: 562
Joined: Thu Sep 11, 2008 8:51 pm
Location: Finland

Re: Line filters not working

Postby matthias1955 » Wed Jun 03, 2009 5:46 pm

I add a patch doing it for GUI only.
Just to show what we can expect. For filecompare we must implement same to diffutils.
For ignoring empty lines: nothing had been changed.
Before it was done in analyze_hunk() with the same problem as we have with linefilter!
(trivial = ignore_blank_lines_flag || ignore_regexp_list)
Only trivial if the whole block is trivial.

So why we should not come to one function for both?
matthias1955
 
Posts: 162
Joined: Wed Dec 17, 2008 1:55 pm

Re: Line filters not working

Postby kimmov » Wed Jun 03, 2009 7:41 pm

And I'm not interested in patches fixing "some" problems. And I really aren't interested in technical details while the problem is in defining how the feature should work. This blank line ignore is non-issue. It works as it was meant to work. If we change it we cause lots of confusion for users who like how it works now. So we don't do it.

If we start altering how filtering works, we must very carefully plan it, think how users can update their filters (best would be to update automatically) and make it very clear for users that now filters behave differently.

Look through feature request tracker to get some idea about expectations and wishes users have. Perhaps we could combine some good feature set from those requests. I would like to combine line filtering and file filtering so that you can apply certain line filters to files matching certain file filter. E.g. you could apply different filters for .cpp files and for .html files. Then we should be able to define per filter if every line, one line or first line (for example) need to match to filter to match the diff. Just couple of two important things.
kimmov
 
Posts: 562
Joined: Thu Sep 11, 2008 8:51 pm
Location: Finland

Re: Line filters not working

Postby kimmov » Wed Jun 03, 2009 8:04 pm

To put it differently: why do you think you can break my currently working filters to fix one bug? If you "fix" something you also break lots of working filters that work with current assumptions. And what is worst, many users have just fixed their filters to work in 2.12. Just to find out that their filters are broken again in 2.14 release. This kind of scenario must not happen.
kimmov
 
Posts: 562
Joined: Thu Sep 11, 2008 8:51 pm
Location: Finland

Re: Line filters not working

Postby matthias1955 » Thu Jun 04, 2009 4:50 pm

I don't like to break your filtering working.
I asked as empty line is allowed on one side. Why it should be differend for linefilter? As we want to ignore a differ here also. That's why I asked if it is a must to have a match for linefilter on both side.
Trivial to do. :)
Otherwise I didn't break anything on current work.
Only I solve the mainproblem, if the diff doesn't match to filter or empty lines complety, I break it into parts what users expect.
sample: linfilter for 'Testme'
a1--------------
a2Testme 1
a3--------------

and file b:
b1--------------
b2something
b3Testme 2
b4--------------
a user expects to see: first a diff insert b2 than a trivial a2/b3
actually he sees a2/b2-b3 as one diff, that's defnitly wrong.
nothing else my patch is doing.
First I though to make filterering than diffs, but that takes a lot more time, as filtering must be done for every
line, not only inside a DIFF. So to splitt a diff is mutch more economically and faster.

so you see my patch has nothing todo with your thinking of redesign on filtering.
matthias1955
 
Posts: 162
Joined: Wed Dec 17, 2008 1:55 pm

Next

Return to Developers

Who is online

Users browsing this forum: No registered users and 1 guest