You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "sebbASF (via GitHub)" <gi...@apache.org> on 2024/05/26 22:21:18 UTC
[PR] Support generation of single quotes [commons-lang]
sebbASF opened a new pull request, #1227:
URL: https://github.com/apache/commons-lang/pull/1227
This is a potential fix to allow for generation of single quotes.
Whilst this changes the behaviour of the class, does it matter?
Would users ever need to create a format string with an empty constant string?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2148354253
Yes, SDF does support that; the problem is that LANG does not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1618741719
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
+ if (val.length() > 0) {
+ buffer.append(val);
+ } else {
+ buffer.append("'");
+ }
Review Comment:
Hello @sebbASF
Or, more succinctly: `buffer.append(val.isEmpty() ? "'" : val);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2149449334
Yes. This PR does handle that specific syntax, i.e. a pair of single-quotes outside a quoted string.
However, SDF also allows such a pair inside a quoted string.
For example "'Joan''s birthday is 'EEE, MMM d, yy"
Currently the embedded pair are treated ending a string and starting a new one.
This PR does not change that, i.e. it does not behave like SDF.
But it does allow one to use the format:
"'Joan''''s birthday is 'EEE, MMM d, yy"
Currently, that is treated 3 separate constant strings (plus date format)
This PR treats the 2nd empty constant string as meaning a lone single quote.
Whilst that allows for a lone quote without breaking existing tests, it does not agree with SDF, which renders the embedded 4 single-quotes as 2 pairs of single-quotes. Existing tests don't include this possibility, but would be broken by the PR.
It is also very awkward to use.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF closed pull request #1227: Support generation of single quotes
URL: https://github.com/apache/commons-lang/pull/1227
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1716867285
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
Review Comment:
OK, then perhaps valueString or valueAsString or something like that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2307908623
This PR was raised as a possible solution which is compatible with *existing* test cases.
It does not make LANG compatible with SDF.
Nor is it a nice solution; the syntax is quite awkward, and may make it harder to achieve SDF compatibility.
I think it should not be applied; rather there should be a move towards SDF compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1618741719
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
+ if (val.length() > 0) {
+ buffer.append(val);
+ } else {
+ buffer.append("'");
+ }
Review Comment:
Or, more succinctly: `buffer.append(val.isEmpty() ? "'" : val);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2148095287
Hm, in the Javadoc for SDF I see:
```
"EEE, MMM d, ''yy" Wed, Jul 4, '01
```
So they DO support doubling single quotes to generate one quote. What am I missing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1716750491
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
Review Comment:
No, rename val to s since it's just a string. In this case there's no obvious difference between val and value and s is pretty standard in Java for arbitrary string variables that don't have a deeper meaning.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2299284400
I'm waiting for some action on this PR in order to merge. I suppose this is not urgent but I am planning a release candidate soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1715940146
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
Review Comment:
val --> s?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2137219153
Good question, this is where we need to make a judgment call on what kind of input the class takes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1716753131
##########
src/test/java/org/apache/commons/lang3/time/DurationFormatUtilsTest.java:
##########
@@ -473,6 +473,21 @@ public void testFormatPeriod() {
assertEquals("helloworld", DurationFormatUtils.formatPeriod(time1970, time, "'hello''world'"));
}
+ @Test
+ public void testSingleQuotes() {
+ final Calendar cal1970 = Calendar.getInstance();
+ cal1970.set(1970, Calendar.JANUARY, 1, 0, 0, 0);
+ cal1970.set(Calendar.MILLISECOND, 0);
+ final long time1970 = cal1970.getTime().getTime();
+ long time = time1970 + 60 * 1000;
+ // empty quoted strings: '' should now generate a single quote
Review Comment:
OK, now I see why I was confused. Two single quotes is **not** or no longer is an empty quoted string. This is an escape sequence for a single quote. Rewrite comment to clarify.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1716816171
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
Review Comment:
The name 's' is already in use in this scope so your comment was confusing to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1716871318
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
Review Comment:
On line 249 what's the actual type? Could we just cast that to string?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#discussion_r1716104837
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -264,7 +264,12 @@ static String format(final Token[] tokens, final long years, final long months,
}
if (isLiteral) {
if (!inOptional || !lastOutputZero) {
- buffer.append(value.toString());
+ final String val = value.toString();
Review Comment:
What do you mean by `val --> s?` ? Assign `val` to `s`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2140935030
Note that this solution, whilst honouring the existing tests, would mean that the syntax deviates from the SimpleDateFormat style. It's not ideal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support generation of single quotes [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1227:
URL: https://github.com/apache/commons-lang/pull/1227#issuecomment-2148849269
Pardon my possible confusion but isn't this PR all about addressing this discrepancy?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org