diff mbox

[BZ,#15527] strftime_l.c: Support lowercase output

Message ID 5847E1D6.5040509@redhat.com
State New
Headers show

Commit Message

Jakub Martisko Dec. 7, 2016, 10:17 a.m. UTC
Oh, my bad. That seems reasonable, sending new patch.
JM

On 7.12.2016 00:20, Mike Frysinger wrote:
> On 29 Jun 2016 09:55, Jakub Martisko wrote:

>> thanks for your comment. The reason why I sent the patch is that there 

>> is a bug/feature request for similar functionality in coreutils' "date" 

>> program and the maintainers of coreutils/gnulib do not want to diverge 

>> from the glibc interface. Even though the replacing you mentioned does 

>> indeed work, built-in version would be imo better (for example when 

>> using other shell than bash), especially when all of the needed 

>> functionality was already implemented.

> 

> i think his point is that bash has already defined a syntax, but you

> are doing it differently and there's (afaict) no need for it.  he isn't

> saying you should use bash if you want lower/upper case.

> 

> so instead of adding new syntax like "%#^x", add "%,x"

> -mike

>

Comments

Mike Frysinger Dec. 7, 2016, 3:39 p.m. UTC | #1
this makes sense to me.  might be good to get wider opinion about new
strftime flags though.
-mike
Rafal Luzynski Dec. 9, 2016, 1:14 a.m. UTC | #2
7.12.2016 16:39 Mike Frysinger <vapier@gentoo.org> wrote:
> [...] might be good to get wider opinion about new

> strftime flags though.

> -mike


Now I lean a little against this change. As I wrote before,
I'd love to see a flag to change the first letter to uppercase
(or to titlecase, if that's more appropriate). A flag to change
whole string to lowercase would be a workaround for this problem
if we also converted all months and weekdays names to the
titlecase in all languages, like in English now. So we would have:

- titlecase by default,
- lowercase if a flag is added explicitly,
- uppercase if a flag is added explicitly.

All reasonable cases covered. Except that converting all locale
data would cause some problems to existing software and that
it wouldn't help the applications which retrieve the months names
or weekdays names with nl_langinfo() rather than strftime().
So indeed I think that a flag to convert to titlecase would
be more useful.

But in that case, do you guys think that converting to lowercase
is useful if all letters are either lowercase already or should
be always as they are now? Can you explain why would any application
ever need the lowercase which should be provided by a format flag
rather than converting programmatically? It seems to be useful
only if some locales want to convert some data to lowercase and
they don't have it lowercase out of the box.

Regards,

Rafal
Mike Frysinger Dec. 9, 2016, 4:07 a.m. UTC | #3
On 09 Dec 2016 02:14, Rafal Luzynski wrote:
> 7.12.2016 16:39 Mike Frysinger wrote:

> > [...] might be good to get wider opinion about new

> > strftime flags though.

> 

> Now I lean a little against this change. As I wrote before,

> I'd love to see a flag to change the first letter to uppercase

> (or to titlecase, if that's more appropriate). A flag to change

> whole string to lowercase would be a workaround for this problem

> if we also converted all months and weekdays names to the

> titlecase in all languages, like in English now.


i don't think changing the locale data or forcing titlecase everywhere
makes sense.  the data is already in the standard format that users
expect for their locale.  having a flag to support that seems like a
good way to get developers to naively enforce their own expectations
onto users.  "my language writes things like 'Dec', therefore every
language out there must use titlecase".

forcing to all uppercase or lowercase seems a bit more reasonable in
certain outputs (everything is upper/lower case beyond the date).

> But in that case, do you guys think that converting to lowercase

> is useful if all letters are either lowercase already or should

> be always as they are now? Can you explain why would any application

> ever need the lowercase which should be provided by a format flag

> rather than converting programmatically? It seems to be useful

> only if some locales want to convert some data to lowercase and

> they don't have it lowercase out of the box.


we already have a flag to force it to uppercase.  makes sense to have
a flag to do the opposite.  your arguments here apply to the uppercase
flag too.
-mike
Rafal Luzynski Dec. 9, 2016, 11 a.m. UTC | #4
9.12.2016 05:07 Mike Frysinger <vapier@gentoo.org> wrote:
>

>

> On 09 Dec 2016 02:14, Rafal Luzynski wrote:

> > 7.12.2016 16:39 Mike Frysinger wrote:

> > > [...] might be good to get wider opinion about new

> > > strftime flags though.

> >

> > Now I lean a little against this change. As I wrote before,

> > I'd love to see a flag to change the first letter to uppercase

> > (or to titlecase, if that's more appropriate). A flag to change

> > whole string to lowercase would be a workaround for this problem

> > if we also converted all months and weekdays names to the

> > titlecase in all languages, like in English now.

>

> i don't think changing the locale data or forcing titlecase everywhere

> makes sense. the data is already in the standard format that users

> expect for their locale. having a flag to support that seems like a

> good way to get developers to naively enforce their own expectations

> onto users. "my language writes things like 'Dec', therefore every

> language out there must use titlecase".


That's really not what I meant. I meant that switching all locale
data to titlecase would be possible only if there was a flag to
switch back to lowercase.

Yes, in my language (and I think that in most languages) we have
all months names and weekdays names in lowercase and this is correct
by default. But we have no flag to switch to titlecase and
I think it would be useful sometimes.

>

> forcing to all uppercase or lowercase seems a bit more reasonable in

> certain outputs (everything is upper/lower case beyond the date).

>

> > But in that case, do you guys think that converting to lowercase

> > is useful if all letters are either lowercase already or should

> > be always as they are now? Can you explain why would any application

> > ever need the lowercase which should be provided by a format flag

> > rather than converting programmatically? It seems to be useful

> > only if some locales want to convert some data to lowercase and

> > they don't have it lowercase out of the box.

>

> we already have a flag to force it to uppercase. makes sense to have

> a flag to do the opposite. your arguments here apply to the uppercase

> flag too.

> -mike


No, does not. I didn't say anything about uppercase, if I had
to say I'd say it's correct so let's leave it as it is now.
What I wanted to say is that we don't have a flag to switch
to titlecase.

My question was: what is the real problem solved by implementing
a flag to switch to lowercase?

And my suggestion is that it would be useful only if a locale
allows all-lowercase months or weekdays but locale data deliver
non-lowercase. For example titlecase. This would solve two problems:
- provide titlecase,
- provide an easy way to generate lowercase.

Regards,

Rafal
Mike Frysinger Dec. 9, 2016, 4:20 p.m. UTC | #5
On 09 Dec 2016 12:00, Rafal Luzynski wrote:
> 9.12.2016 05:07 Mike Frysinger wrote:

> > On 09 Dec 2016 02:14, Rafal Luzynski wrote:

> > > 7.12.2016 16:39 Mike Frysinger wrote:

> > > > [...] might be good to get wider opinion about new

> > > > strftime flags though.

> > >

> > > Now I lean a little against this change. As I wrote before,

> > > I'd love to see a flag to change the first letter to uppercase

> > > (or to titlecase, if that's more appropriate). A flag to change

> > > whole string to lowercase would be a workaround for this problem

> > > if we also converted all months and weekdays names to the

> > > titlecase in all languages, like in English now.

> >

> > i don't think changing the locale data or forcing titlecase everywhere

> > makes sense. the data is already in the standard format that users

> > expect for their locale. having a flag to support that seems like a

> > good way to get developers to naively enforce their own expectations

> > onto users. "my language writes things like 'Dec', therefore every

> > language out there must use titlecase".

> 

> That's really not what I meant. I meant that switching all locale

> data to titlecase would be possible only if there was a flag to

> switch back to lowercase.


i'm saying the flags are irrelevant.  the data should be in the form
that is appropriate for that locale.  that might mean uppercase, or
lowercase, or titlecase.  flags would not help at all, and in fact,
they would make things worse.  en_US authors would see that, think
that it's weird looking, and then use the titlecase flag.  then it'd
be broken for many other locales.

> Yes, in my language (and I think that in most languages) we have

> all months names and weekdays names in lowercase and this is correct

> by default. But we have no flag to switch to titlecase and

> I think it would be useful sometimes.


i have a hard time seeing a valid usecase where the code forcing
titlecase all the time would be desirable.  forcing all lower or
upper case isn't that hard to imagine (e.g. output forms where
*all* content, not just dates, are in the respective cases).

> > forcing to all uppercase or lowercase seems a bit more reasonable in

> > certain outputs (everything is upper/lower case beyond the date).

> >

> > > But in that case, do you guys think that converting to lowercase

> > > is useful if all letters are either lowercase already or should

> > > be always as they are now? Can you explain why would any application

> > > ever need the lowercase which should be provided by a format flag

> > > rather than converting programmatically? It seems to be useful

> > > only if some locales want to convert some data to lowercase and

> > > they don't have it lowercase out of the box.

> >

> > we already have a flag to force it to uppercase. makes sense to have

> > a flag to do the opposite. your arguments here apply to the uppercase

> > flag too.

> 

> No, does not. I didn't say anything about uppercase, if I had

> to say I'd say it's correct so let's leave it as it is now.

> What I wanted to say is that we don't have a flag to switch

> to titlecase.

> 

> My question was: what is the real problem solved by implementing

> a flag to switch to lowercase?


i didn't say you talked about uppercase.  my point is that your argument
isn't specific to lowercase.  you could s/lower/upper/ and have the same
points.  so why do you think having uppercase makes sense but adding
lowercase is wrong ?

> And my suggestion is that it would be useful only if a locale

> allows all-lowercase months or weekdays but locale data deliver

> non-lowercase. For example titlecase. This would solve two problems:

> - provide titlecase,

> - provide an easy way to generate lowercase.


locale data cannot be changed.  adding flags to transform their case
would not help the situation at all, just make it worse.
-mike
Rafal Luzynski Dec. 9, 2016, 10:37 p.m. UTC | #6
9.12.2016 17:20 Mike Frysinger <vapier@gentoo.org> wrote:
>

>

> On 09 Dec 2016 12:00, Rafal Luzynski wrote:

> > [...]

> > That's really not what I meant. I meant that switching all locale

> > data to titlecase would be possible only if there was a flag to

> > switch back to lowercase.

>

> i'm saying the flags are irrelevant. the data should be in the form

> that is appropriate for that locale. that might mean uppercase, or

> lowercase, or titlecase. flags would not help at all, and in fact,

> they would make things worse. en_US authors would see that, think

> that it's weird looking, and then use the titlecase flag. then it'd

> be broken for many other locales.


Format strings should be translatable and it should be left to
translators what format string and what flags they want. An application
should present all messages, format strings, and date/time data
either in en_US or all should be localized, otherwise we
get some unfinished mix of localized/not-yet-localized.
So although it's correct to use titlecase in English it should not
be relevant for other languages.

> > Yes, in my language (and I think that in most languages) we have

> > all months names and weekdays names in lowercase and this is correct

> > by default. But we have no flag to switch to titlecase and

> > I think it would be useful sometimes.

>

> i have a hard time seeing a valid usecase where the code forcing

> titlecase all the time would be desirable.


No, I didn't mean a code forcing titlecase all the time.  My aim
is to provide a choice: lowercase/titlecase/uppercase, for each
format specifier individually.  For now in English (and German)
we have titlecase/uppercase, in other languages we have lowercase/uppercase.
What is a solution to provide the missing case?

> forcing all lower or

> upper case isn't that hard to imagine (e.g. output forms where

> *all* content, not just dates, are in the respective cases).


I agree with the uppercase, but is it really useful to have
all lowercase in English?

> > > [...]

> > > we already have a flag to force it to uppercase. makes sense to have

> > > a flag to do the opposite. your arguments here apply to the uppercase

> > > flag too.

> >

> > No, does not. I didn't say anything about uppercase, if I had

> > to say I'd say it's correct so let's leave it as it is now.

> > What I wanted to say is that we don't have a flag to switch

> > to titlecase.

> >

> > My question was: what is the real problem solved by implementing

> > a flag to switch to lowercase?

>

> i didn't say you talked about uppercase. my point is that your argument

> isn't specific to lowercase. you could s/lower/upper/ and have the same

> points. so why do you think having uppercase makes sense but adding

> lowercase is wrong ?


Because we already have all data in lowercase.  No need to convert
from lowercase to lowercase.  More precisely: in some languages
the data are in titlecase but should remain so, converting them
to lowercase is incorrect.

My point is not that lowercase flag is wrong or destructive,
I just think it does not provide any additional value.

A separate problem is that there is no flag to convert to
titlecase.  My suggestion is that if we had locale data in
titlecase (like in English) and a flag to convert them to
lowercase explicitly then it would be a workaround.  Of course
a titlecase flag would be better.  But, again, that's a separate
case, only similar to the lowercase flag.

Regards,

Rafal
Rafal Luzynski April 7, 2017, 11:29 p.m. UTC | #7
Hi,

This thread has been started almost one year ago [1] but has been
inactive for last 4 months. [2] What is its current progress? Do you
guys think about continuing this work and eventually committing it
to the main repo?

I think that my previous comments could be misunderstood. I didn't
mean that the patch is bad, I just wonder what is its application
and whether Jakub, the author, can see any and whether it is the
same application as I see.

Regards,

Rafal


[1] https://sourceware.org/ml/libc-alpha/2016-05/msg00259.html
[2] https://sourceware.org/ml/libc-alpha/2016-12/msg00034.html
diff mbox

Patch

From 1289b48ee174bda5dc6266df1ed216f172fc5dd4 Mon Sep 17 00:00:00 2001
From: Jakub Martisko <jamartis@redhat.com>
Date: Wed, 7 Dec 2016 10:52:08 +0100
Subject: [PATCH] time/stfrtime_l.c: add lowercase support

* [BZ #15527]
* time/strftime_l.c (__strftime_internal): Implement conversion to
all lowercase.
* manual/time.texi: Document # and , flags.
* time/tst-strftime.c (do_test): Test case conversion.
---
 manual/time.texi    |  8 ++++++++
 time/strftime_l.c   |  4 +++-
 time/tst-strftime.c | 30 ++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/manual/time.texi b/manual/time.texi
index f94cbe4..39ed062 100644
--- a/manual/time.texi
+++ b/manual/time.texi
@@ -1352,6 +1352,14 @@  with spaces.
 @item ^
 The output uses uppercase characters, but only if this is possible
 (@pxref{Case Conversion}).
+
+@item #
+The output uses opposite case, but only if this is possible
+(@pxref{Case Conversion}).
+
+@item ,
+The output uses lowercase characters, but only if this is possible
+(@pxref{Case Conversion}).
 @end table
 
 The default action is to pad the number with zeros to keep it a constant
diff --git a/time/strftime_l.c b/time/strftime_l.c
index 1205035..6ed5f63 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -671,7 +671,9 @@  __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 	    case L_('#'):
 	      change_case = 1;
 	      continue;
-
+      case L_(','):
+  	      to_lowcase = 1;
+  	      continue;
 	    default:
 	      break;
 	    }
diff --git a/time/tst-strftime.c b/time/tst-strftime.c
index af3ff72..258d40a 100644
--- a/time/tst-strftime.c
+++ b/time/tst-strftime.c
@@ -154,6 +154,36 @@  do_test (void)
 	}
     }
 
+	const struct
+	  {
+	    const char *fmt;
+	    const char *exp;
+	    size_t n;
+	  } ctests[] =
+	    {
+	      { "%^A", "SUNDAY", 6 },
+	      { "%,A", "sunday", 6 },
+	      { "%A", "Sunday", 6 },
+	    };
+#define nctests (sizeof (ctests) / sizeof (ctests[0]))
+	  for (cnt = 0; cnt < nctests; ++cnt)
+    {
+      char buf[100];
+      size_t r = strftime (buf, sizeof (buf), ctests[cnt].fmt, &ttm);
+      if (r != ctests[cnt].n)
+	{
+	  printf ("strftime(\"%s\") returned %zu not %zu\n",
+		  ctests[cnt].fmt, r, ctests[cnt].n);
+	  result = 1;
+	}
+      if (strcmp (buf, ctests[cnt].exp) != 0)
+	{
+	  printf ("strftime(\"%s\") produced \"%s\" not \"%s\"\n",
+		  ctests[cnt].fmt, buf, ctests[cnt].exp);
+	  result = 1;
+	}
+    }
+
   return result + do_bz18985 ();
 }
 
-- 
2.5.5