Return statments fix
AbandonedPublic

Authored by rade on Mar 20 2018, 6:15 PM.

Details

Reviewers
nmel
Summary

I have changed the code to return true when we have handled/filtered an event. Reason being that the spec explicitly says to do that to prevent any other code also handling the event.

In your reimplementation of this function, if you want to filter the event out, i.e. stop it being handled further, return true; otherwise return false.

There used to be a problem where folders where triggered when the first letter of their name was typed which I believed was because the event was handled by some other code, and that it was fixed by returning true. I can't replicate the incorrect behaviour with 42858f0e though so it's possible I was mistaken.

Test Plan

Ensure there have been no regressions.

Verify that all code paths lead to the correct return value.

Diff Detail

Repository
R167 Krusader
Lint
Lint Skipped
Unit
Unit Tests Skipped
rade requested review of this revision.Mar 20 2018, 6:15 PM
rade created this revision.
nmel added a comment.Mar 21 2018, 5:39 AM

I'm confused regarding how to apply this as returns has been partially covered by D11525 (except for the break replacement). Do you want to include it there?

rade abandoned this revision.EditedMar 21 2018, 12:46 PM
In D11521#230503, @nmel wrote:

I'm confused regarding how to apply this as returns has been partially covered by D11525 (except for the break replacement). Do you want to include it there?

You requested in D11443 that I split it into three different revisions so this was my attempt at that. I probably shouldn't have included the return statements in D11525 or made it a patch to be applied on top of this patch instead. I'm not quite sure what the right thing to have done would have been. I'll merge this back into D11525 and split my code changes correctly in the future.

nmel added a comment.Mar 22 2018, 6:51 AM
In D11521#230782, @rade wrote:

I probably shouldn't have included the return statements in D11525 or made it a patch to be applied on top of this patch instead. I'm not quite sure what the right thing to have done would have been.

Yes, I expected return statements only here and D11525 to be applied on top of this patch.

I'll merge this back into D11525 and split my code changes correctly in the future.

Great, it works for me, thanks a lot!