Ticket #3996 (closed defect: fixed)

Opened 5 years ago

Last modified 5 years ago

Dynamic paragraphing no longer usable

Reported by: JRottm Owned by: andrew_b
Priority: minor Milestone: 4.8.24
Component: mcedit Version: 4.8.22
Keywords: dynamic paragraphing Cc: egmont
Blocked By: Blocking:
Branch state: approved Votes for changeset: committed-master

Description

To reproduce:
1) open a new, empty file in mcedit
2) Options->General: enable Dynamic paragraphing. Hint: also set line length e.g. to 20, makes the next step faster, because you don't have to type so much.
3) Type rubbish words and occasional spaces e.g. "lorem ipsum dolor sit amet..." or whatever until you have filled 2-4 lines.
4) Start a new paragraph by pressing enter twice.
5) Repeat until you have 2-3 paragraphs.
6) Move cursor back to 1st paragraph.
7) Try to edit something in the 1st paragraph.

Expected behavior:
You can edit 1st paragraph. Lines should wrap dynamically if you add or delete words.

Observed behaviour:
After just 1 keypress (added or deleted letter) cursor immediately jumps to start of next paragraph. If you had intended to add a whole word in 1st paragraph you have to move cursor back, add another letter, move cursor back, add another letter, etc. This makes "dynamic paragraphing" utterly unusable.

Affected versions:

  • Debian 10 = mc 4.8.22 is broken
  • Debian 9 = mc 4.8.18 was broken
  • Debian 8 = mc 4.8.13 worked fine, if I remember correctly

My current system:
Debian 10, x86, 64 bit.
GNU Midnight Commander 4.8.22
Built with GLib 2.58.2
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;
mc --configure-options: '--build=x86_64-linux-gnu' '--prefix=/usr' '--includedir=${prefix}/include' '--mandir=${prefix}/share/man' '--infodir=${prefix}/share/info' '--sysconfdir=/etc' '--localstatedir=/var' '--libdir=${prefix}/lib/x86_64-linux-gnu' '--libexecdir=${prefix}/lib/x86_64-linux-gnu' '--runstatedir=/run' '--disable-maintainer-mode' '--disable-dependency-tracking' 'AWK=awk' 'X11_WWW=x-www-browser' '--libexecdir=/usr/lib' '--with-x' '--with-screen=slang' '--disable-rpath' '--disable-static' '--disable-silent-rules' '--enable-aspell' '--enable-vfs-sftp' '--enable-vfs-undelfs' '--enable-tests' 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -fdebug-prefix-map=/build/mc-4.8.22=. -fstack-protector-strong -Wformat -Werror=format-security' 'LDFLAGS=-Wl,-z,relro -Wl,-z,now -Wl,--as-needed' 'CPPFLAGS=-Wdate-time -D_FORTIFY_SOURCE=2'

Thanks, and best regards

Change History

comment:1 Changed 5 years ago by zaytsev

  • Cc egmont added

Would be really awesome if you could bisect this... /cc egmont just because it faintly remember you did a lot of work on the editor / viewer back in the time.

comment:2 Changed 5 years ago by andrew_b

This is a result of #1666.

comment:3 Changed 5 years ago by andrew_b

Do we want move to the next paragraph after format of current one?

comment:4 Changed 5 years ago by egmont

I worked on the viewer only, so that's not relevant here.

comment:5 follow-up: ↓ 6 Changed 5 years ago by JRottm

Hi again, turns out memory decieved me. Debian 8 wasn't fine, but Debian 7 (4.8.3) was. Always bugged me, should've reported it earlier...

Downloaded bin packages from Ubuntu and Slackware to narrow it down:

  • 4.8.5 (Ubuntu, May 2013): works fine
  • 4.8.10 (Slackware, Oct 2013): Dyn. Para. has zero effect at all, completely nonfunctional
  • 4.8.12 (Ubuntu, Apr 2014): Dyn. Para. is back, but with bug as described above

I really can't see how this could be intentional, you can't edit a text if your cursor keeps jumping away. Even when you're already in the last paragraph the cursor jumps to the end of that after each letter. In its current form Dyn. Para. is useless except when adding to or deleting from the very end of the last paragraph.

I guess I could attempt to bisect, however it's obvious that this is not just 1 unlucky commit, some kind of rework took place 4.8.5→10, which then someone tried to fix 4.8.10→12, and only got it almost right. And even if I find the (at least) 2 commits in question, I doubt you'll be able to revert them, the code surely has changed too much. So if you ask me to bisect, I will, but only if you think it will actually help you.

Thanks & regards

comment:6 in reply to: ↑ 5 Changed 5 years ago by andrew_b

Replying to JRottm:

So if you ask me to bisect, I will, but only if you think it will actually help you.

I've already found the bad commit (see comment:2).

I would like to get a reply to my question in comment:3:

Do we want move to the next paragraph after format of current one?

comment:7 follow-up: ↓ 8 Changed 5 years ago by JRottm

IMHO current behavior makes no sense. If one's intention was to only format all paragraphs, one after the other, why would you have to add/delete a letter in each to trigger the formatting. If someone wants batch-formatting they'd just filter it through fold(1). Rather, it's supposed to wrap dynamically while you're editing your text, but you can't edit when the cursor keeps jumping away.

comment:8 in reply to: ↑ 7 Changed 5 years ago by egmont

If someone wants batch-formatting they'd just filter it through fold(1).

Nitpicking: There's a function for this in mcedit: F9 -> Format -> Format paragraph (M-p). This action should (and does) jump to the beginning of the next paragraph, so that you can simply repeat it multiple times (or hold M-p) to get it repeated for multiple paragraphs.

Rather, it's supposed to wrap dynamically while you're editing your text, but you can't edit when the cursor keeps jumping away.

I fully agree. While typing (or erasing) text, the cursor needs to stay at the same logical position within the text during automatic formatting, so that (apart from space <-> newline conversions) the text ends up being whatever you type. Otherwise this functionality is useless.

Two side notes:

As I'm testing this feature now, I face other buggy behavior as well. There's nothing concrete I could reproduce and report at this moment, but I've faced a line getting wrapped much sooner than desired; also a complete line getting erased from the display, as part of a bigger display corruption where each line got fixed as soon as the cursor entered it.

If this feature has been broken for 5-6 years without anyone reporting it, it's pretty much a sign that hardly anyone wishes to have this mode. Instead of fixing it, removing it for good is another possibility to consider. I mean, OP would sure be disappointed by this resolution, but from the project's maintenance point of view it might make sense. Note: I'm not saying this is what the project should do, I'm just saying it might be a reasonable step to consider. (Do popular text editors, e.g. vim and emacs have such a mode?)

Last edited 5 years ago by egmont (previous) (diff)

comment:9 Changed 5 years ago by andrew_b

The simple fix is revert of [ccb7ab341e775892f05c0ded3dce0423c6aab585] and reopen of #1666.

If we do that, dynamic formatting gets working again. But if paragraph is formatted, dynamic formatting is not working.

comment:10 Changed 5 years ago by andrew_b

  • Owner set to andrew_b
  • Status changed from new to accepted
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.24

Branch: 3996_dynamic_format_fix
changeset:6b21b8f4e67635f993b7ec0895398a3a1e9bef59

comment:11 Changed 5 years ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:12 Changed 5 years ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed

comment:13 Changed 5 years ago by andrew_b

  • Status changed from testing to closed

comment:14 Changed 5 years ago by andrew_b

#1666 reopened.

Note: See TracTickets for help on using tickets.