Re: [Bug] \~ can overwrite previous text.

[Alejandro Colomar]

Hi Branden,

On 3/6/23 18:46, G. Branden Robinson wrote:
> You're close but not quite there, I _think_.  (Until I've root-caused
> this myself, or seen someone else's convincing demonstration, my
> conclusions must be tentative ones.)
> 
> Because _ordinary_ spaces don't get squeezed, I would start at
> `distribute_space()` in src/roff/troff/env.cpp and navigate up and down
> the call stack from there.
> 
> static void distribute_space(node *n, int nspaces, hunits desired_space,
>                              bool force_reverse_node_list = false)

Thanks!  This seems to be involved.  I tried a trivial fix to see if it
would work.  Sadly, it has negative consequences (a regression).  I'll
share so you don't lose your time in the same attempt:


diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 9f00284c6..0ec4e40e3 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -2081,11 +2081,12 @@ static node *node_list_reverse(node *n)
 static void distribute_space(node *n, int nspaces, hunits desired_space,
                             bool force_reverse_node_list = false)
 {
-  if (desired_space.is_zero() || nspaces == 0)
-    return;
   // Positive desired space is the typical case.  Negative desired space
-  // is possible if we have overrun an unbreakable line.  But we should
-  // not get here if there are no adjustable space nodes to adjust.
+  // is possible if we have overrun an unbreakable line.
+  if (desired_space.to_units() <= 0 || nspaces == 0)
+    return;
+  // But we should not get here if there are no adjustable space nodes
+  // to adjust.
   assert(nspaces > 0);
   // Space cannot always be distributed evenly among all of the space
   // nodes in the node list: there are limits to device resolution.  We
@@ -2098,8 +2099,7 @@ static void distribute_space(node *n, int nspaces, hunits 
desired_space,
   static bool do_reverse_node_list = false;
   if (force_reverse_node_list || do_reverse_node_list)
     n = node_list_reverse(n);
-  if (!force_reverse_node_list && spread_limit >= 0
-      && desired_space.to_units() > 0) {
+  if (!force_reverse_node_list && spread_limit >= 0) {
     hunits em = curenv->get_size();
     double Ems = (double)desired_space.to_units() / nspaces
                 / (em.is_zero() ? hresolution : em.to_units());



With the above patch, my bug is fixed, but the following test fails:

    XFAIL: tmac/tests/e_footnotes-work-with-columns.sh

which is about <https://savannah.gnu.org/bugs/?58736>.
Now, I'm not sure if the fix correct and the old #58736 needs further
fixing, or if I should try to find a better fix that doesn't cause
regressions.  I'll investigate a little bit more.


Cheers,

Alex


-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

OpenPGP_signature
Description: OpenPGP digital signature