Ticket #2135 (closed defect: fixed)

Opened 9 years ago

Last modified 8 years ago

End button in MC viewer jumps too far

Reported by: birdie Owned by: slavazanko
Priority: major Milestone: 4.7.5
Component: mcview Version: 4.7.1
Keywords: Cc: egmont@…, gotar@…
Blocked By: Blocking:
Branch state: Votes for changeset: commited-master

Description

I don't know if it's already been reported but since MC 4.7.1 it became terribly inconvenient to jump to the end of any file in mcviewer: end button jumps such a way the only and last line of a file is visible, so you have to scroll back a little to actually see a file tail.

If you try to tail view dozens of files, scrolling back each one of them becomes a very tedious task.

Attachments

mcview.png (18.2 KB) - added by birdie 9 years ago.
With "mcview_eof=~" set.
mc-down2.diff (1.8 KB) - added by gotar 9 years ago.
mcview_move_down.patch
mc-down3.diff (1.8 KB) - added by andrew_b 8 years ago.

Change History

comment:1 Changed 9 years ago by marrtins

  • Type changed from defect to enhancement

Yes, this is annoying, please fix! At least now viewer scrolls down fast with large files too.

comment:2 follow-up: ↓ 3 Changed 9 years ago by angel_il

marrtins, birdie
please show version of mc

$ mc -V

birdie, set
mcview_eof=~ in your ini file to make easier to see the end of file

comment:3 in reply to: ↑ 2 Changed 9 years ago by birdie

mc-4.7.1-2.fc13.i686

Replying to angel_il:

marrtins, birdie
please show version of mc

$ mc -V

birdie, set
mcview_eof=~ in your ini file to make easier to see the end of file

That didn't help at all, mc still jumps beyond the end of file and shows a lot of ~, see the attached screenshot.

Changed 9 years ago by birdie

With "mcview_eof=~" set.

comment:4 Changed 9 years ago by marrtins

My version:

GNU Midnight Commander 4.7.1
Virtual File System: tarfs, extfs, cpiofs, ftpfs, fish
With builtin Editor
Using system-installed S-Lang library with terminfo database
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With internationalization support
With multiple codepages support
Data types: char 8 int 32 long 32 void * 32 off_t 64 ecs_char 8

comment:5 Changed 9 years ago by egmont

  • Cc egmont@… added

+1 for fixing it please! This is a very annoying regression in the just-to-be-released stable 4.7.2.

In order to see as much as fits on the screen from the end of the file, instead of pressing End now I have to press End, then Pageup, then Down. Or End and then longpress the Up key.

The behavior of the End key should be the same as in the panels: the last line of the file should appear on the bottom of the available area; just as the last file appears in the bottom row, with lots of other filenames also visible in the mean time.

comment:6 Changed 9 years ago by andrew_b

There are some non-trivial problems with this issue. We would not fix this problem very quickly and finally get a slow viewer in some cases.

comment:7 Changed 9 years ago by slavazanko

  • Status changed from new to accepted
  • Owner set to slavazanko
  • severity changed from no branch to on review

Well... created branch 2135_viewer_jump_to_eof

Changesets:

Review, please.

comment:8 Changed 9 years ago by egmont

These patches work nice for me so far. Thanks very much guys!

comment:9 Changed 9 years ago by andrew_b

  • Votes for changeset set to andrew_b
  • Milestone changed from 4.7 to 4.7.2

comment:10 Changed 9 years ago by angel_il

  • Votes for changeset changed from andrew_b to ossi andrew_b angel_il
  • severity changed from on review to approved

comment:11 Changed 9 years ago by slavazanko

  • Keywords stable-candidate added
  • Status changed from accepted to testing
  • Votes for changeset changed from ossi andrew_b angel_il to committed-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:12 Changed 9 years ago by slavazanko

  • Status changed from testing to closed

comment:13 follow-up: ↓ 14 Changed 9 years ago by egmont

Thanks very much guys, especially for making it into 4.7.2!

Quick question: are you maybe considering backporting the second part of the patch to the stable 4.7.0.x branch? PageDown? was able to scroll up too much, ever since my earliest MC memories, and finally it no longer happens, hooray! Nice usability improvement!

comment:14 in reply to: ↑ 13 Changed 9 years ago by angel_il

Replying to egmont:

Thanks very much guys, especially for making it into 4.7.2!

Quick question: are you maybe considering backporting the second part of the patch to the stable 4.7.0.x branch? PageDown? was able to scroll up too much, ever since my earliest MC memories, and finally it no longer happens, hooray! Nice usability improvement!

I do not know and am not sure whether you need to backport new viewer into 4.7.0.х

comment:15 Changed 9 years ago by egmont

Oh, the whole viewer is new? I thought it would be just a trivial merge. Forget it then :)

comment:16 Changed 9 years ago by slavazanko

Yes, for stable we have old slow viewer. For 'master' branch viewer was rewritten.

comment:17 Changed 9 years ago by gotar

  • Status changed from closed to reopened
  • Type changed from enhancement to defect
  • Resolution fixed deleted
  • Cc gotar@… added

With this patch you've broken one of the old viewer features (for me, see #1778). So I've got hopefully compromise proposition for the first part, which also addresses two of three issues I've found:

  1. 175 view->dpy_end = last_byte;

it's not true: from line 159 condition - 'visible part' (view->dpy_end - view->dpy_start) larger than 'remaining part' (last_byte - view->dpy_end) doesn't mean we'll end up at the end, as it depends at least on forward newline count we don't know. Oh, and it is being set at 168.

  1. 180 for (i = 0; i < lines && view->dpy_end < last_byte && new_offset < last_byte; i++)

we don't track view->dpy_end so it's pointless to check this condition, the value will be updated later.

  1. once again - condition from 159th line won't handle the case, when lines below currently visible area contain more than is displayed (just imagine 100 empty lines followed by some long ones, especially with disabled wrap mode) - I left this untouched, as it's more rare case and resolving it requires tracking view->dpy_end everytime.

What I suggest is fixing PgDn in a way, that it switches to one-line mode after EOF and add a check to ensure, that at least one last line stays visible (4.7.1 behaviour of end/pgdn was irritating indeed).

Changed 9 years ago by gotar

mcview_move_down.patch

comment:18 Changed 9 years ago by angel_il

  • Milestone changed from 4.7.2 to 4.7.3

comment:19 Changed 9 years ago by angel_il

  • Milestone changed from 4.7.3 to 4.7.4

comment:20 Changed 8 years ago by gotar

  • severity changed from merged to on rework
  • Milestone changed from 4.7.4 to 4.7.5

Two slight changes to my patch:

  1. leave 'off_t new_offset = 0;'
  2. 'if(view->dpy_end>=last_byte) break;' instead 'lines=0' (and move to the end of the loop).

Changed 8 years ago by andrew_b

comment:21 follow-up: ↓ 23 Changed 8 years ago by andrew_b

  • Votes for changeset committed-master deleted

I've attached new patch.

With this pathch, PageDown? at the last page scrolls file line by line instead of entire page.

comment:22 Changed 8 years ago by slavazanko

  • Status changed from reopened to accepted
  • Keywords stable-candidate removed
  • severity changed from on rework to on review

With this pathch, PageDown? at the last page scrolls file line by line instead of entire page.

In my point of view, this should be feature... ;)

Created branch 2135_end_button_in_viewer
changeset:525320071c661c874295fa6dbb83317bb0d9ebfa

review, please.

comment:23 in reply to: ↑ 21 ; follow-up: ↓ 24 Changed 8 years ago by birdie

Replying to andrew_b:

I've attached new patch.

With this pathch, PageDown? at the last page scrolls file line by line instead of entire page.

IMO when we've reached the end of file, MC just shouldn't do anything if I try to scroll down even more. Scrolling line by line will most likely confuse people ("WTF is my MC doing?")

comment:24 in reply to: ↑ 23 Changed 8 years ago by gotar

Replying to birdie:

IMO when we've reached the end of file, MC just shouldn't do anything if I try to scroll down even more. Scrolling line by line will most likely confuse people ("WTF is my MC doing?")

And IMO it should be possible to scroll so high as it was possible to do earlier: #1778. Switching to 1-line mode prevents user from immediately scrolling off entire content, and notices him about file coming to the end (if he doesn't see percentage indicator).

comment:25 Changed 8 years ago by slavazanko

  • Blocking 2411 added

comment:26 Changed 8 years ago by andrew_b

  • Votes for changeset set to andrew_b

comment:27 Changed 8 years ago by slavazanko

  • Votes for changeset changed from andrew_b to andrew_b slavazanko
  • severity changed from on review to approved

patch made by gotar, therefore my vote here.

comment:28 Changed 8 years ago by slavazanko

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b slavazanko to commited-master
  • Resolution set to fixed
  • severity changed from approved to merged
  • Blocking 2411 removed

comment:29 Changed 8 years ago by slavazanko

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.