fill sometimes fails (seemingly if word after break is quite long), seems a regression in 4.16

[markmoraes]

foo bar bleep special terminal, you should not need termcap/terminfo/curses/ncurses/ncursesw.

The above line does not wrap on fill-paragraph with
a right-margin >= 51 (i.e. right-margin-here on
the space after "need" If the right-margin is 50,
i.e. right-margin-here on the final character 'd'
of "need", then it correctly fills)

This fails on 4.16.0.74 and 4.17 head, seems to work on 4.14.11 (a tiny set of quite minimal changes -- rename getline -> jgetline, fix errno declaration -- to build 4.14.10 on Linux, to give us a very old baseline for this sort of regression)

[markmoraes]

Looks like this problem appeared in from the following change, made in

+#define        jversion     "4.16.0.74" /* 2015 October 15 */

for which I cannot find a record in the RCS that Hugh sent me in Jan 2020, but which I see email about from that date, "Subject: test fire: jove 4.16.0.74", announcing ftp://ftp.cs.toronto.edu/pub/hugh/jove-dev/experimental/jove4.16.0.74.tgz

The relevant change in paragraph.c, which, if reversed, fixes the problem (but presumably re-introduces whatever problem this change was intended to fix) is:

@@ -477,6 +491,12 @@
                while (!eolp() && !jiswhite(linebuf[curchar]))
                        curchar += 1;
 
+               /* stop if we've run out of range */
+               if (curline == endmark->m_line && curchar >= endmark->m_char) {
+                       curchar = endmark->m_char;
+                       break;
+               }
+
                if (word_start != curchar && okay_char != start_char
                && calc_pos(linebuf, curchar) > RMargin) {
                        /* This non-empty word won't fit in output line
@@ -504,9 +524,6 @@
                } else {
                        /* this word fits (it might be empty, but that's OK) */
                        okay_char = curchar;    /* nail down success */
-                       /* stop if we've run out of range */
-                       if (curline == endmark->m_line && curchar >= endmark->m_char)
-                               break;
 
                        /* process word separator */
                        if (eolp() && !lastp(curline)) {

[markmoraes]

Fairly sure that #18 is a duplicate of this. I think I understand the issue (but hey, it's the Jove paragraph code, which, um, is like definitely in the top ten code paths in Jove that scare me). The problem with the test for out-of-range being before the if() is that it triggers when curchar reaches the end of the para, but it cannot break out of the loop yet because curchar may also be past RMargin and should wrap that last word. It could be that the test for out-of-range should be before the preceding skip-forward-a-word loop, or, as the prior code did, in the body of the else -- I'm tempted towards the latter because while there may well be some situation where that is slightly wrong, we clearly lived with it for many years before 4.16.0.74, so it can't have been too horribly wrong (in the core dumping or memory trashing or text mangling sense?!)

[markmoraes]

And the file is now para.c in 4.17 (probably my renaming to bring most files under 8 chars if possible!)

[markmoraes]

didn't mean to close, leaving open for further comments or test cases where the older behavior was wrong. @HughR