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.