subcommanderblog.wordpress.com

Subcommander Development Blog

Improving submerge wheel scrolling

leave a comment »

submerge (subcommanders diff and merge tool) uses the mouse wheel to quickly jump from one difference to the next or the previous difference. Let’s call this jump scrolling.

To achieve this I handle the mouse wheel event in the parent widget of the of the widget that really displays the files. To do normal scrolling, i.e. line by line, one has to move the mouse above the scroll bar which handles the mouse wheel in the usual way.

Sometimes this is a bit annoying if you just like to scroll a few lines up or down to review the visible difference. You move the wheel and … ups, you are at another difference in the file, maybe several pages from the difference you just reviewed. So the idea is to allow normal scrolling by using a keyboard modifier. Maybe I will even switch the default, and change jump scrolling to shift + wheel.

I’m currently reading “Clean Code” from Robert C. Martin, although it doesn’t offer new revolutionary ideas, it is always good to be remembered of things you already know but don’t follow consistently.  So let’s see if I can also improve the readability of the code, at least a bit :-).

Here is the code as it was before. A few notes about the code:

  • Diff3Widget is the parent widget that shows all versions of the file we diff  (old file, new file etc.).
  • _diffInfo is a member that holds the actual diff information.
  • the diff information contains several block. A block represents a number of equal lines in the two file or a number lines that are different in both files. That’s the reason why diff number and block number are not the same. Hmm, not sure if this is clear ;-)

 

void Diff3Widget::wheelEvent( QWheelEvent* e )
{
  if( ! _diffInfo )
  {
    e->ignore();
    return;
  }

  //printf( "wheel delta: %d (%p)\n", e->delta(), e );

  int b = 0;

  if( e->delta() > 0 )
  {
    // forward, down
    if( _diffInfo->hasPrevDiff() )
    {
      b = _diffInfo->prevDiff();
    }
  }
  else
  {
    // backward, up
    if( _diffInfo->hasNextDiff() )
    {
      b = _diffInfo->nextDiff();
    }
  }

  if( b == 0 )
  {
    e->ignore();
    return;
  }

  jumpToBlock( b );
  setActiveDiff( _diffInfo->getActiveDiff() );

  emit diffChanged( _diffInfo->getActiveDiff() );

  e->accept();
}

 

I ended up with the code below. It uses two members that display the files in submerge (_single is the original file (the third file) if any and _double which contains the left and right file) to do the normal scrolling. I also dropped the _diffInfo pointer check. I had already a null implementation for the diff info and I now initialize the member with it so I do not have to check for a NULL pointer.

 

void Diff3Widget::wheelEvent( QWheelEvent* e )
{
  if( shouldScrollNormal(e->modifiers()) )
    scrollNormal(e->delta());
  else
    scrollToDiff(e->delta());

  e->accept();
}

void Diff3Widget::scrollNormal( int delta )
{
  QScrollBar* sbSingle = _single->getVScrollBar();
  QScrollBar* sbDouble = _double->getVScrollBar();
  sbSingle->setValue(sbSingle->value()-delta);
  sbDouble->setValue(sbDouble->value()-delta);
}

bool Diff3Widget::shouldScrollNormal( Qt::KeyboardModifiers modifiers )
{
  if( modifiers == Qt::ShiftModifier )
    return true;

  if( _diffInfo->getDiffCnt() == 0 )
    return true;

  return false;
}

void Diff3Widget::scrollToDiff( int delta )
{
  if( delta > 0 )
    scrollToPrevDiff();
  else
    scrollToNextDiff();
}

void Diff3Widget::scrollToPrevDiff()
{
  if( !_diffInfo->hasPrevDiff() )
    return;

  jumpToBlock( _diffInfo->prevDiff() );
  setActiveDiff( _diffInfo->getActiveDiff() );
}

void Diff3Widget::scrollToNextDiff()
{
  if( !_diffInfo->hasNextDiff() )
    return;

  jumpToBlock( _diffInfo->nextDiff() );
  setActiveDiff( _diffInfo->getActiveDiff() );
}

 

I split up the original method into several methods and reduced the code in each method to what is interesting at that level. Clearly better than before but I think there are still a few things you can suggest to improve the code. Anything?

What’s missing is some test code… how would you test this?

Advertisements

Written by hauner

Saturday, 24 January, 2009 at 19:17

Posted in submerge

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: