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

[Alejandro Colomar]

On 3/7/23 02:15, Alejandro Colomar wrote:
> 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.

Another patch which fixes my bug and breaks the same footnote thingy:


diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 9f00284c6..e7724fe30 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -2106,8 +2106,10 @@ static void distribute_space(node *n, int nspaces, 
hunits desired_space,
     if (Ems > spread_limit)
       output_warning(WARN_BREAK, "spreading %1m per space", Ems);
   }
-  for (node *tem = n; tem; tem = tem->next)
-    tem->spread_space(&nspaces, &desired_space);
+  if (desired_space.to_units() > 0) {
+    for (node *tem = n; tem; tem = tem->next)
+      tem->spread_space(&nspaces, &desired_space);
+  }
   if (force_reverse_node_list || do_reverse_node_list)
     (void)node_list_reverse(n);
   if (!force_reverse_node_list)



> 
> 
> Cheers,
> 
> Alex
> 
> 

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

OpenPGP_signature
Description: OpenPGP digital signature