Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ This opens a window that lets you view the files' pages and zoom in on details.
It is also possible to shift the two pages relatively to each other using
Ctrl-arrows (Cmd-arrows on MacOS). This is useful for identifying translation-only differences.

You can also use Ctrl < and Ctrl > (Cmd < and Cmd > on MacOS) show the left and right documents respecively. Ctrl d to return back to the diff view.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the standard way of referring to combined keypresses, which is Ctrl+D etc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the update. I tried staying consistent with the existing convention in the code. Perhaps this should be fixed also for the other combined key-presses for the zoom and offset:

        toolbar->AddTool(ID_ZOOM_IN, "Zoom in", BMP_ZOOM_IN,
                         "Make the page larger (Ctrl +)");
        toolbar->AddTool(ID_ZOOM_OUT, "Zoom out", BMP_ZOOM_OUT,
                         "Make the page smaller (Ctrl -)");
        toolbar->AddTool(ID_OFFSET_LEFT, "", BMP_OFFSET_LEFT,
                         "Offset one of the pages to the left (Ctrl left)");
        toolbar->AddTool(ID_OFFSET_RIGHT, "", BMP_OFFSET_RIGHT,
                         "Offset one of the pages to the right (Ctrl right)");
        toolbar->AddTool(ID_OFFSET_UP, "", BMP_OFFSET_UP,
                         "Offset one of the pages up (Ctrl up)");
        toolbar->AddTool(ID_OFFSET_DOWN, "", BMP_OFFSET_DOWN,
                         "Offset one of the pages down (Ctrl down)");


See the output of `$ diff-pdf --help` for complete list of options.


Expand Down
75 changes: 72 additions & 3 deletions diff-pdf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@
#include <wx/progdlg.h>
#include <wx/filesys.h>


enum DisplayMode
{
SHOW_DIFF_DOCUMENT,
SHOW_LEFT_DOCUMENT,
SHOW_RIGHT_DOCUMENT
};

// ------------------------------------------------------------------------
// PDF rendering functions
// ------------------------------------------------------------------------
Expand Down Expand Up @@ -537,6 +545,9 @@ const int ID_OFFSET_RIGHT = wxNewId();
const int ID_OFFSET_UP = wxNewId();
const int ID_OFFSET_DOWN = wxNewId();
const int ID_GUTTER = wxNewId();
const int ID_LEFT_DOC = wxNewId();
const int ID_RIGHT_DOC = wxNewId();
const int ID_DIFF_DOC = wxNewId();

#define BMP_ARTPROV(id) wxArtProvider::GetBitmap(id, wxART_TOOLBAR)

Expand Down Expand Up @@ -597,11 +608,17 @@ class DiffFrame : public wxFrame
"Offset one of the pages up (Ctrl up)");
toolbar->AddTool(ID_OFFSET_DOWN, "", BMP_OFFSET_DOWN,
"Offset one of the pages down (Ctrl down)");
toolbar->AddTool(ID_LEFT_DOC, "Left document", BMP_ARTPROV(wxART_GO_BACK),
"Show left document (Ctrl <)");
toolbar->AddTool(ID_DIFF_DOC, "Diff document", BMP_ARTPROV(wxART_REDO),
"Show diff document (Ctrl d)");
toolbar->AddTool(ID_RIGHT_DOC, "Right document", BMP_ARTPROV(wxART_GO_FORWARD),
"Show right document (Ctrl >)");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These icons seem arbitrary — and semantically incorrect.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am open to suggestions regarding the icons. I wanted something to indicate the left document (left arrow) right document (right arrow) and diff (I am using the redo arrow as its sort of returning to the default comparison mode)

I am not familiar with wx icons so if you have better suggestions I am open to change this :)


toolbar->Realize();
SetToolBar(toolbar);

wxAcceleratorEntry accels[8];
wxAcceleratorEntry accels[11];
accels[0].Set(wxACCEL_NORMAL, WXK_PAGEUP, ID_PREV_PAGE);
accels[1].Set(wxACCEL_NORMAL, WXK_PAGEDOWN, ID_NEXT_PAGE);
accels[2].Set(wxACCEL_CTRL, (int)'=', ID_ZOOM_IN);
Expand All @@ -610,8 +627,11 @@ class DiffFrame : public wxFrame
accels[5].Set(wxACCEL_CTRL, WXK_RIGHT, ID_OFFSET_RIGHT);
accels[6].Set(wxACCEL_CTRL, WXK_UP, ID_OFFSET_UP);
accels[7].Set(wxACCEL_CTRL, WXK_DOWN, ID_OFFSET_DOWN);
accels[8].Set(wxACCEL_CTRL, (int)',', ID_LEFT_DOC);
accels[9].Set(wxACCEL_CTRL, (int)'.', ID_RIGHT_DOC);
accels[10].Set(wxACCEL_CTRL, (int)'d', ID_DIFF_DOC);

wxAcceleratorTable accel_table(8, accels);
wxAcceleratorTable accel_table(11, accels);
SetAcceleratorTable(accel_table);

m_gutter = new Gutter(this, ID_GUTTER);
Expand Down Expand Up @@ -695,8 +715,32 @@ class DiffFrame : public wxFrame
m_offset.x, m_offset.y,
&thumbnail, Gutter::WIDTH
);
// Render page according to the current display mode
switch ( m_display_mode )
{
case SHOW_LEFT_DOCUMENT:
//
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh?

if ( img1 )
m_viewer->Set(img1);
else if ( img2 )
m_viewer->Set(img2);
else
m_viewer->Set(NULL);
break;

case SHOW_RIGHT_DOCUMENT:
if ( img2 )
m_viewer->Set(img2);
else if ( img1 )
m_viewer->Set(img1);
else
m_viewer->Set(NULL);
break;
Comment on lines 727 to 743
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t understand why this code shows left-side image on the right side and vice versa?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my head img1 is the Left document and img2 is the right document.

The idea is if we are in the SHOW_RIGHT_DOCUMENT mode, we try showing the right document, however if the image doesn't exist (if the documents aren't the same length, we just show the left document. Seemed to make most sense in my usecase scenario, however I could change this to just display some kind of default image (like Page, not available....)


m_viewer->Set(diff ? diff : img1);
case SHOW_DIFF_DOCUMENT:
default:
m_viewer->Set(diff ? diff : img1);
}

// Always update the diff map. It will be all-white if there were
// no differences.
Expand Down Expand Up @@ -788,6 +832,27 @@ class DiffFrame : public wxFrame
DoUpdatePage();
}

void OnShowLeftDocument(wxCommandEvent&)
{
m_display_mode = SHOW_LEFT_DOCUMENT;
m_offset = wxPoint(0, 0);
DoUpdatePage();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these reset the offset — seems unrelated?

Please also don’t duplicate code like this, use a helper function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, there should be a dedicated button (shortcut for this)

In my use case scenario I found it usefult to use the the Ctrl+D shortcut to not only return to the the diff mode but also the reset the offset. I removed it in the update

}

void OnShowDiffDocument(wxCommandEvent&)
{
m_display_mode = SHOW_DIFF_DOCUMENT;
m_offset = wxPoint(0, 0);
DoUpdatePage();
}

void OnShowRightDocument(wxCommandEvent&)
{
m_display_mode = SHOW_RIGHT_DOCUMENT;
m_offset = wxPoint(0, 0);
DoUpdatePage();
}

void OnOffsetLeft(wxCommandEvent&) { DoOffset(-1, 0); }
void OnOffsetRight(wxCommandEvent&) { DoOffset(1, 0); }
void OnOffsetUp(wxCommandEvent&) { DoOffset(0, -1); }
Expand All @@ -803,6 +868,7 @@ class DiffFrame : public wxFrame
int m_diff_count;
int m_cur_page;
wxPoint m_offset;
DisplayMode m_display_mode;
};

BEGIN_EVENT_TABLE(DiffFrame, wxFrame)
Expand All @@ -817,6 +883,9 @@ BEGIN_EVENT_TABLE(DiffFrame, wxFrame)
EVT_TOOL (ID_OFFSET_RIGHT, DiffFrame::OnOffsetRight)
EVT_TOOL (ID_OFFSET_UP, DiffFrame::OnOffsetUp)
EVT_TOOL (ID_OFFSET_DOWN, DiffFrame::OnOffsetDown)
EVT_TOOL (ID_LEFT_DOC, DiffFrame::OnShowLeftDocument)
EVT_TOOL (ID_DIFF_DOC, DiffFrame::OnShowDiffDocument)
EVT_TOOL (ID_RIGHT_DOC, DiffFrame::OnShowRightDocument)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong indentation, D is not aligned.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the indent.... would you like me to set up a clang-format to keep the style consistent?

END_EVENT_TABLE()


Expand Down
Loading