Re: code improvements for the viewer

[Roland Illig <roland illig gmx de>]

Pavel Tsekov wrote:
> Just out of curiosity - can you outline the most important changes that
> you are going to commmit.

The main change is that, for example in the view_display_text() 
function, coordinate computation had been done relative to the top left 
corner of the WView widget. One consequence was that the 
view_display_text() and view_display_hex() function had to know whether 
the status line is displayed (it always is) and which kind of ruler is 
active (try Alt-R in mcview; it can be invisible, at the top or at the 
bottom).

These calculations have been factored out into the function 
view_compute_areas(). Now while calculating the coordinates for where to 
display characters, (0,0) always means the upper left corner of the 
current area (which is one of view->status_area, view->ruler_area and 
view->data_ares). Only when calling view_gotoyx(), the (top,left) 
coordinates of each area are considered.

An example of code that gets much simpler is this:

view_display_text():
-           col = ((col - left) / 8) * 8 + 8 + left;
+           col = col - col % 8 + 8;

The new code just advances the column to the next tab stop. The old code 
first had to subtract the left coordinate of the viewer and had to add 
it at the end. Even worse, some revisions before it had not been called 
``left'', but ``frame_size''.

Another thing is that many coordinate calculations were done 
incorrectly, but with valid results:

view_percent():
-    const int xpos = view->widget.cols - view->dpy_frame_size - 4;
+    const screen_dimen xpos = view->status_area.left
                             + view->status_area.width - 4;

The point is, the old code relies on properties of the complete widget 
(width and frame size), whereas the new code only needs data that has to 
do with the status area itself. That is, the code expresses what is 
really meant, and not something coincidentally equivalent.

Similarly, in view_status():
-    widget_move (view, view->dpy_frame_size,
                        view->dpy_frame_size + 24);
+    widget_move (view, top, left + 24);

In view_display_ruler(), there had been an off-by-one error which only 
occured in panel mode (that is: view->dpy_frame_size != 0):

-    for (c = left; c < right; c++) {
-       cl = c + view->dpy_text_column;
+    for (c = 0; c < width; c++) {
+       cl = view->dpy_text_column + c;

cl is the column in the current line (it is >= 0 if you scroll to the 
right). In the old code, it took into account whether the data was 
displayed with or without a frame. While separating the real calculation 
and the coordinate thing, I noticed that bug. It would have been hard to 
notice in the old code, as the calculation were more complex than necessary.

That's a small part of the ideas underlying my changes. If I had written 
everything, it would have been a book on programming style. :)

Roland