Use strftime_l() instead of global setlocale() for %{!...}

[nabijaczleweli]

New ltrace for normal locale-ful format (same as before):

gmtime_r(1691438423, 0x7fff5c6b6400)                                                             = {23, 0, 20, 7, 7, 123, 1, 218, 0, 0, "GMT"}
strftime("sie 07", 1024, "%b %d", {23, 0, 20, 7, 7, 123, 1, 218, 0, 0, "GMT"})                   = 6

New ltrace for the first locale-free format:

gmtime_r(1691438423, 0x7fff5c6b63c0)                                                             = {23, 0, 20, 7, 7, 123, 1, 218, 0, 0, "GMT"}
duplocale(0xffffffffffffffff)                                                                    = 0x55c07ca0a6b0
newlocale(0b100, "C", 0x55c07ca0a6b0)                                                            = 0x55c07c8e7b60
strftime_l("Aug 07", 1024, "%b %d", {23, 0, 20, 7, 7, 123, 1, 218, 0, 0, "GMT"}, 0x55c07c8e7b60) = 6

and for subsequent ones:

gmtime_r(1691438422, 0x7fff5c6b63c0)                                                             = {22, 0, 20, 7, 7, 123, 1, 218, 0, 0, "GMT"}
strftime_l("Aug 07", 1024, "%b %d", {22, 0, 20, 7, 7, 123, 1, 218, 0, 0, "GMT"}, 0x55c07c8e7b60) = 6

[flatcap]

Hmm... this reply has got quite complicated as I investigated...
Whatever you've got time for would be appreciated.
(ask lots of questions :-)
Thanks!

---

OK, this seems reasonable; we already use strftime_l() in one place.
I'd like you to go a bit further please, and fix something that's irritating me :-)

First, the irritation...
Since Mutt's first commit to revision control, over twenty years ago, there've been local variables: do_locales.
They're set, then only checked in the negative: if (!do_locales)
Please can you invert them to bool use_c_locale / if (use_c_locale)

Looking through the code, there are only a handful of calls to strftime().
Plus, a dozen calls to each of mutt_date_localtime_format() and mutt_make_string().
Many of those are wrapped by setlocale(LC_TIME, "C");
There's one other thing to consider: $attribution_locale.

As we need the C locale in quite a few places, I suggest adding it to struct NeoMutt.
That way we can free it on exit (unlike a static variable).

Fixing mutt_date_localtime_format() may just be a case of creating a new 'C locale' function.

Fixing mutt_make_string() will be a bit more work.
We'd need to pass the locale in and change struct HdrFormatInfo.

Places of interest:

mutt/date.c:930:10:           return strftime(buf, buflen, format, &tm);

browser/dlg_browser.c:251:11: setlocale(LC_TIME, "C");
browser/dlg_browser.c:255:11: setlocale(LC_TIME, "");
browser/dlg_browser.c:310:11: setlocale(LC_TIME, "C");
browser/dlg_browser.c:311:9:  strftime(buf2, sizeof(buf2), buf, &tm);
browser/dlg_browser.c:313:11: setlocale(LC_TIME, "");

hdrline.c:714:11:             setlocale(LC_TIME, "C");
hdrline.c:715:9:              strftime(tmp, sizeof(tmp), buf, &tm);
hdrline.c:717:11:             setlocale(LC_TIME, "");

ncrypt/dlg_gpgme.c:553:9:     setlocale(LC_TIME, "C");
ncrypt/dlg_gpgme.c:554:7:     strftime(buf2, sizeof(buf2), buf, &tm);
ncrypt/dlg_gpgme.c:556:9:     setlocale(LC_TIME, "");

ncrypt/dlg_pgp.c:471:9:       setlocale(LC_TIME, "C");
ncrypt/dlg_pgp.c:474:9:       setlocale(LC_TIME, "");

mx.c:1121:22:                 locale_t loc = newlocale(LC_TIME_MASK, "C", 0);
mx.c:1122:7:                  strftime_l(buf, sizeof(buf), "%a %b %e %H:%M:%S %Y", &tm, loc);
mx.c:1123:7:                  freelocale(loc);
mx.c:1125:7:                  strftime(buf, sizeof(buf), "%a %b %e %H:%M:%S %Y", &tm);

copy.c:669:7:                 setlocale(LC_TIME, NONULL(c_attribution_locale));
copy.c:672:7:                 setlocale(LC_TIME, "");
recvcmd.c:423:7:              setlocale(LC_TIME, NONULL(c_attribution_locale));
recvcmd.c:426:7:              setlocale(LC_TIME, "");
recvcmd.c:529:7:              setlocale(LC_TIME, NONULL(c_attribution_locale));
recvcmd.c:532:7:              setlocale(LC_TIME, "");
recvcmd.c:1038:7:             setlocale(LC_TIME, NONULL(c_attribution_locale));
recvcmd.c:1041:7:             setlocale(LC_TIME, "");
send/send.c:456:3:            setlocale(LC_TIME, NONULL(c_attribution_locale));
send/send.c:459:3:            setlocale(LC_TIME, "");
send/send.c:479:3:            setlocale(LC_TIME, NONULL(c_attribution_locale));
send/send.c:482:3:            setlocale(LC_TIME, "");
send/send.c:641:3:            setlocale(LC_TIME, NONULL(c_attribution_locale));
send/send.c:643:3:            setlocale(LC_TIME, "");

browser/dlg_browser.c:231:14: bool do_locales = true;
browser/dlg_browser.c:241:13: do_locales = false;
browser/dlg_browser.c:250:14: if (!do_locales)
browser/dlg_browser.c:254:14: if (!do_locales)
browser/dlg_browser.c:269:14: bool do_locales = true;
browser/dlg_browser.c:277:11: do_locales = false;
browser/dlg_browser.c:309:14: if (!do_locales)
browser/dlg_browser.c:312:14: if (!do_locales)
hdrline.c:633:14:             bool do_locales;
hdrline.c:636:11:             do_locales = false;
hdrline.c:641:11:             do_locales = true;
hdrline.c:713:14:             if (!do_locales)
hdrline.c:716:14:             if (!do_locales)
ncrypt/dlg_gpgme.c:509:12:    bool do_locales = true;
ncrypt/dlg_gpgme.c:517:9:     do_locales = false;
ncrypt/dlg_gpgme.c:552:12:    if (!do_locales)
ncrypt/dlg_gpgme.c:555:12:    if (!do_locales)
ncrypt/dlg_pgp.c:432:12:      bool do_locales = true;
ncrypt/dlg_pgp.c:440:9:       do_locales = false;
ncrypt/dlg_pgp.c:470:12:      if (!do_locales)
ncrypt/dlg_pgp.c:473:12:      if (!do_locales)

[nabijaczleweli]

Okay, I got rid of all the LC_TIMEs except for the attribution_locale ones (also fixed the test which described "i set TZ=UTC0" as "travis' locale problems") – I think the most sensible way to do it would be to register a notification handler for c_attribution_locale and maintain it as a locale_t, but AFAICT the handlers are for "subsets"? Can I register a global one? Where would I put it?

[flatcap]

I've only had a quick look; I'll have a closer read later.

NeoMutt->sub is the "global" subset.
Here's a complicated diagram "explaining" the config system

[nabijaczleweli]

$ git grep LC_TIME
copy.c:  if (!(new = newlocale(LC_TIME_MASK, loc, NeoMutt->time_attribution_locale)))
core/neomutt.c:  if (!(n->time_C_locale = newlocale(LC_TIME_MASK, "C", n->time_C_locale)))
core/neomutt.h:  locale_t time_C_locale;           ///< Current locale but LC_TIME=C
core/neomutt.h:  locale_t time_attribution_locale; ///< Current locale but LC_TIME=$attribution_locale
docs/config.c:** accepts for the locale environment variable \fC$$$LC_TIME\fP.

[nabijaczleweli]

rebased

[flatcap]

merged two trivial commits

done

[flatcap]

I've merged the C locale parts. Thanks!

I changed just one thing.
mutt_date_localtime_format_c() became mutt_date_localtime_format_locale().
By passing in the locale, we avoid adding a libcore dependency to libmutt.

[flatcap]

I not quite decided on the remaining attribution_locale commit.

Caching another config variable seems a bit like overkill.
Yes, there are plenty of places where it's used, but none of them are time-critical, like the index.

[nabijaczleweli]

the obverse is doing the whole freelocale(newlocale(LC_TIME_MASK, "de_EN.UTF-8", duplocale(LC_GLOBAL_LOCALE))) (+ error handling (bad)) dance in every user, for every formatted string

so I think "the config changes when the config changes" is much simpler, faster, and easier, than copy-pasting the same code 6 times. i'm actually surprised the solution to changing the config is "wham it in a random global" but I guess having a structured config would be unpalatable to retrofit in C

[flatcap]

Archived to https://github.com/neomutt/neomutt-old/tree/attribution