-- Christoffer Holmstedt 2013/6/19 Christoffer Holmstedt <christoffer.holmstedt@...400...> > Right...forgot my reason for including it =/ ...this time around it is > just about one function "sp_toggle_dropper". A similiar function which > currently resides next to it in event-context.cpp is "sp_toggle_selector" > (spacebar switch, to and back from select tool). If I ever search the code > for any of these two features in future I would assume they would be next > to each other, therefore I left "sp_toggle_dropper" where it is. Moving > both outside is of course an option but as "sp_toggle_selector" is private > I didn't find that as compelling. > > Aha, yea. Thanks for clearing that up, makes sense that most of them are > probably pretty hard to fix. > > -- > Christoffer Holmstedt > > > 2013/6/19 Jon Cruz <jon@...18...> > >> >> On Jun 17, 2013, at 11:11 PM, Christoffer Holmstedt wrote: >> >> > My second bug fix is available now at >> https://bugs.launchpad.net/inkscape/+bug/180912 it builds and works just >> fine but I have one question. I had to include "event-context.h" in >> "verbs.cpp" and I'm not sure if this coupling is wise to do. Any comments >> on that? >> >> Well... the main question would be as to why you had to include it. If it >> is to access aspects of all contexts no matter what kind, then it might be >> appropriate. On the other hand, if it was to access just a helper or >> function from that header, it might be good to split such functionality out >> >> >> > I didn't want to move the special toggling feature for the dropper tool >> to verbs.cpp mainly due to the space toggling to "select tool" and back >> would still be in event-context.cpp. >> > >> > During compilation there are a few "unused variable" warnings and other >> kinds of warnings as well. Within the Inkscape project do you usually file >> a bug report against this (for keeping track of stuff) and then fix the >> warning(s) or just fix it? >> >> Normally just fix them as you encounter them. If feasible, try to do such >> fixes in a separate check-in. That will help keep the history clear. >> >> However... I've usually cleaned up the straightforward unused variables, >> etc., so any that are still in there might be complicated to fix, require >> knowledge of the specific code area (some of the more complex maths parts >> get like this), or might be due to #ifdefs that make them used for some >> configurations and unused for others. >> >> >

-- Christoffer Holmstedt 2013/6/25 Martin Owens <doctormo@...400...> > On Tue, 2013-06-25 at 08:38 +0200, Christoffer Holmstedt wrote: > > Bump. If no one has any more comments about it I would be interested > > in getting this patch merged to trunk. Is there any other formal > > process to be accepted as Inkscape developer? > > > Go to: > > > https://code.launchpad.net/~christoffer-holmstedt/inkscape/lp180912-remove-h... > > Press "Propose for Merging", make sure you target the trunk. > > Martin, > > >