Code search - Shift + drag object

Previous topic - Next topic

Gibbz

Hi,

Ive just posted a patch to add Alt selection mode to Scribus (https://bugs.scribus.net/view.php?id=15477).
Now I want to fix the shift select mode, but I cant seem to find the code for this.

Currently if a frame is selected, and the user holds shift, then begins a click and drag operation, the object moves on the screen. This is the incorrect behavior, and I cant find this code anywhere to look into it.

Ive been looking in canvasmode_normal.cpp in the mousepress, mouserelease, mousemove, but that code is a real mess without comments.

Any help where I can look?

a.l.e

first, congratulations on the patch. good idea!

concerning that "mess". some time ago, i've written to the mailing list a proposal for new code / contributing conventions for scribus.
it has been "officially" ignored, but it led the team to write down a clear list of code conventions that simply cemented the current rules.
(the page is not official yet, so i don't want to publish the link in here. i can share it with you in private.)

why i'm writing about it, now?
i'd like to stress one rule that i think applies to your patch: if a patch adds more than 8 lines in a function, the author should think about refactoring that part of code (and create a new function).
this should also apply to adding inline comments: if you feel the need for inline comments, you should refactor your code, use good names, and put the comment in the doc of the function (if it's still needed).

i had a look at the code in mouseReleaseEvent and in my eyes, there are good changes that the big if following craig's "Drag selection performed here" comment (line 1146?) should get into its own function (onDragSelection()? what are the arguments? what will be the used as class variables? personally, i like to reduce the amount of side effects...).
if possible, the function should be further split... 8 to 10 lines is probably the optimal length for a function. 50 lines probably it's maximal length.

now, having said that, while having had a look at the code, i've noticed a "m_doc->m_Selection->count() == 0" in the big if on line 1147... i think that this is the reason why we cannot extend the current selection through a drag selection.
it's just a supposition: i don't know anything about that part of code and i haven't tried it out...
what do yo uthin?

a.l.e

and now to anecdotes:

i have probably  never tried to use the shift selection area to extend an existing election in scribus... until last monday... and was indeed surprised that it does not work. the biggest issue being that instead of having selected the item the "old" one got moved away... and it was not really easy to spot what was happening.

a second anecdote: i used to have a hard time with inkscape's shift-select tool... it took me some time to get used to it... but while i was trying the alt-select area in scribus to see how it would feel, i was missing the red path... and this even if i've almost never used that feature : - )
... i think that the inkscape folks did this spot right... even if it's a surprising solution (to me)...

Gibbz

Cool, yeah I think ill have a bit more of a play around in that code and I guess just start with commenting it all out and breaking it into smaller functions. That should help with the need for commenting too I believe!

A quick question. Ive changed QT to use tabs, but it still doesnt use them. So i had to fix my patch manually. Is Tabs working on QT Creator for you?

a.l.e

i don't get the tabs to work either... i'm asking in the qt creator irc channel but go no answer yet...

a.l.e