D4981: patchreview : allow choice of the number of context lines

[René J.V. Bertin]

rjvbb added a comment.


  It adds a bit of complexity, but it looks worse than it is if you ask me. It's in fact not much more than a single additional argument; the complexity stems more from the number of places where that argument has to be added than from the changes themselves.
  
  In turn that stems from the complexity of the whole underlying design. It's not the first time I set out to make what I thought would be a simple change was annoyed to find I needed to make way more changes in way more hard to find places than you'd expect. It was indeed my plan to test the whole idea only with git, but I couldn't see a way to put the spinbox where it is now, make it trigger a patch review update and access its information only in the git plugin. Besides, the feature should probably be implemented for the other VCS flavours supported by Phab, at least svn.
  
  Maybe we could use the occasion to change the API to pass the arguments that have default values via a pointer to a class or struct which can hopefully be defined completely in the base class?
  
  Or maybe this new parameter can be implemented as a member variable in `IBasicVersionControl` rather than as a function argument? The class already has a setter for another property so it isn't purely abstract, and this would mean a lot less passing around of an additional variable. I'll have a look at that approach, see if it makes the patch less complex.

REVISION DETAIL
  https://phabricator.kde.org/D4981

To: rjvbb, #kdevelop
Cc: apol, kdevelop-devel, #kdevelop, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170310/8d7eb05c/attachment.html>