bugGNU troff - Bugs: bug #59973, [PATCH] implement .soquiet and...

 
 

bug #59973: [PATCH] implement .soquiet and .msoquiet requests

Submitted by:  G. Branden Robinson <gbranden>
Submitted on:  Sat 30 Jan 2021 06:42:25 AM UTC  
 
Category:  Core Severity:  1 - Wish
Item Group:  Feature change Status:  Fixed
Privacy:  Public Assigned to:  G. Branden Robinson <gbranden>
Open/Closed:  Closed Planned Release:  1.23.0

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Fri 09 Apr 2021 03:59:32 AM UTC, comment #6: 

commit 955323df9ebfb8e0889f8eac77edd3477333fe66
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Fri Apr 9 13:09:31 2021 +1000

    Implement new .soquiet and .msoquiet requests.

    Needed for two planned developments: (1) the i18n patch in Savannah
    bug #59814 can use this to quietly attempt to open a localization file.
    groff only supports a few locales, so people with LANG=es_ES, for
    instance, would ordinarily see warning diagnostics on every groff
    startup if file warnings were enabled; and (2) system- or
    site-configurable support of per-user {.,}troffrc or man.local files,
    gracefully failing if they don't exist.

    * src/roff/troff/input.cpp:
      (do_source): Add new function, taking boolean "quietly" parameter and
      absorbing function of source(), with added conditional.

      (source): Convert into a wrapper to call do_source() unquietly.

      (source_quietly): Wrap do_source(), quietly.

      (do_macro_source, macro_source, macro_source_quietly): Analogous to
      the foregoing.

      (init_requests): Hook "msoquiet" to macro_source_quietly() and
      "soquiet" to "source_quietly().

    * doc/groff.texi (I/O):
    * man/groff.7.man (Requests/Request short reference):
    * man/groff_diff.7.man (Language/New requests): Document them.

    * src/roff/groff/tests/msoquiet_works.sh:
    * src/roff/groff/tests/soquiet_works.sh: Test them.

    * src/roff/groff/groff.am (groff_TESTS): Add tests.

    Fixes <https://savannah.gnu.org/bugs/?59973>.

G. Branden Robinson <gbranden>
Project AdministratorIn charge of this item.
Fri 09 Apr 2021 01:31:42 AM UTC, comment #5: 

Hi Dave,

comment #3:

> Your "while" (non)loop expressed in pseudocode is:
>
>       while (1) {
>        if (A)
>          if (B)
>            break;
>        error(...);
>        break;
>       }
>
> Neither "if" having an "else," this should be equivalent to:
>
>       while (1) {
>        if (A && B)
>          break;
>        error(...);
>        break;
>       }
>
> This further seems to be equivalent to the lower-level pseudocode:
>
>        if (A && B)
>          goto NEXT;
>        error(...)
>        goto NEXT;
>       NEXT:
>
> which basically reduces to:
>
>       if !(A && B)
>         error(...)
>
> I confess I'm no C++ coder, so I could be overlooking any number of language-specific gotchas.  And I don't know what you mean by "complicating the boolean conditional afterward," the thing you were trying to avoid.  But naively tracing the code path, I don't understand the need for the "while" at all.


You are completely correct.  I suspect, and an indentation error on the same reinforces this, that I had something even more complicated originally.

I've refactored to:

if (!(quietly && (ENOENT == errno)))

One could of course apply De Morgan's Law here, but I think the intent is clearer this way.

As you can see, I've also chosen not to worry about EACCES at this point.  This facilitates further simplification of the logic--and, as you note, that error might indicate something actually unexpected.

If we ever need to handle other POSIX errno symbols, I would extend the request to take a list of such strings.  In fact, .so and .mso themselves could be extended in this way... But I do not do so at present because I don't know if there is demand for such a feature.  This one has 2 practical use cases as noted in the original ticket.

Next, update the documentation for these new requests.

G. Branden Robinson <gbranden>
Project AdministratorIn charge of this item.
Thu 18 Mar 2021 01:15:09 AM UTC, comment #4: 

comment #2:

> Specifically, I'm a bit conflicted over whether to ignore
> EACCES [sic] as well as ENOENT, or ignore only ENOENT errors.


I have no strong opinion about this.  I think having the requests in place is the main hurdle to clear.  Tweaking the warning conditions, should that become necessary in the future, is easy at that point.

For the use case you have in mind for this, ENOENT is sufficient, right?  For possible future use cases no one has thought of yet, we can't make any solid predictions, but it seems like warning about EACCES could clue the user in that, hey, maybe not everything is as you thought.

Dave <barx>
Project Member
Fri 05 Mar 2021 06:15:48 AM UTC, comment #3: 

Your "while" (non)loop expressed in pseudocode is:

      while (1) {
       if (A)
         if (B)
           break;
       error(...);
       break;
      }

Neither "if" having an "else," this should be equivalent to:

      while (1) {
       if (A && B)
         break;
       error(...);
       break;
      }

This further seems to be equivalent to the lower-level pseudocode:

       if (A && B)
         goto NEXT;
       error(...)
       goto NEXT;
      NEXT:

which basically reduces to:

      if !(A && B)
        error(...)

I confess I'm no C++ coder, so I could be overlooking any number of language-specific gotchas.  And I don't know what you mean by "complicating the boolean conditional afterward," the thing you were trying to avoid.  But naively tracing the code path, I don't understand the need for the "while" at all.

Dave <barx>
Project Member
Wed 10 Feb 2021 07:52:20 AM UTC, comment #2: 

Specifically, I'm a bit conflicted over whether to ignore EACCES [sic] as well as ENOENT, or ignore only ENOENT errors.

G. Branden Robinson <gbranden>
Project AdministratorIn charge of this item.
Wed 10 Feb 2021 07:51:37 AM UTC, comment #1: 

Feedback desired on whether people think:

(1) This is a good idea;
(2) It's a sound design;
(3) It's implemented reasonably.

G. Branden Robinson <gbranden>
Project AdministratorIn charge of this item.
Sat 30 Jan 2021 06:42:25 AM UTC, original submission:  

Use case #1: The internationalization patch in bug #59814 could use this to quietly attempt to open a localization file.  groff only supports a few locales, so people with LANG=es_ES, for instance would ordinarily see warning diagnostics on every groff startup if file warnings are enabled.

Use case #2: Support of user-specific troffrc or man.local files, gracefully failing if they don't exist or if there is a permissions problem.  Imagine:

.soquiet \v[XDG_DATA_HOME]/man.local

Yes, .msoquiet could be aped with *roff arithmetic to effectively mask off the warning type and then restoring it.  This is incredibly tedious.  It is also fragile, being subject to breakage if a new warning category is ever added; see bug #52990.

Attaching straw-man implementation.  Includes test, does not yet include updates to documentation.

The `while (1)` may look weird.  I judged it better than complicating the boolean conditional afterward (indeed, I managed to mess it up when I tried), and it avoids using goto.  In my opinion it's easier to understand than golfier alternatives I can imagine.

G. Branden Robinson <gbranden>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #50807:  soquiet.diff added by gbranden (6KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by gbranden (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-04-09 gbranden StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
        Planned ReleaseNone => 1.23.0
    2021-04-09 gbranden StatusNeed Info => In Progress
    2021-02-11 gbranden StatusIn Progress => Need Info
    2021-01-30 gbranden Attached File- => Added soquiet.diff, #50807

    Back to the top


    Powered by Savane 3.7