Activating Documents with a range is broken since d77c94c1402916f8f00b02104f71187330548ecd

Andreas Pakulat apaku at gmx.de
Wed Dec 12 22:28:35 UTC 2012


Hi,

On Wed, Dec 12, 2012 at 8:37 PM, Milian Wolff <mail at milianw.de> wrote:
> On Wednesday 12 December 2012 20:25:01 Andreas Pakulat wrote:
>> just bisected why double-clicking entries in the grepview does not
>> work anymore as it should. Turns out that
>> DocumentController::activateDocument( doc, range ) does not jump to
>> the range anymore, unless the document is not open yet. Anybody
>> understands the logic thats involved and can fix that? Otherwise I'll
>> revert this over the weekend.
>>
>> That also makes me wonder wether we maybe should reconsider all of the
>> refactorings that Aleix did in this area, this is not the first
>> regression it caused and the main benefit of lazily initializing views
>> is not usable at the moment either (see my contextmenu related mail)?
>
> I'd also consider that this second breakage means the changes hit master too
> soon. Please revert them, put them into a branch, and lets investigate it
> there.

I'm not sure this would help the intended changes. If it lives in a
branch somewhere almost nobody will test the changes since most
master-users wouldn't want to try experimental stuff. Without people
using the branch on a daily basis for their daily work you're not
going to hit the issues we've seen so far. If you're afraid of having
regressions because you want to push out a 4.5 asap, then you
should've put in a freeze a month ago or so IMO.

I think the best way at the moment to test such changes in the core is
putting them into master to get as many testers as possible. The only
viable alternatives i see are to get all active developers and a
couple of users to use a staging branch locally where they merge all
currently-active feature branches and report back any issues they find
(and which branches they merged locally). Or alternatively to have
unit-tests and ui tests for these things and make it dead-easy to add
new ones. Neither of the two seem to be possible in the short-term.

I'm also not much of a fan of reverting changes, especially when so
many have been done with others in between and potential later changes
that affected the changed code as well. Reverting this requires quite
some effort as well to avoid introducing yet more regressions.

So I'd say, lets try to hammer out any further possible regressions,
maybe re-evaluate the commits by putting them into reviewboard or just
reviewing them locally to see what could possibly be affected by them
and then test that out more. At least its still rather easy to bisect
these and find the culprit for a bug if its related to the
document/view changes and thus get a clue where the bug could hide.


More information about the KDevelop-devel mailing list