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

 
 

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

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

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

( 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 Administrator
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 Administrator
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 Administrator
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 Administrator
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 Administrator

 

(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)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    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.9