[GitHub] commons-lang issue #345: add jvmLaunchers
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/345 It does sound like this is in the Commons Exec domain. ---
[GitHub] commons-lang issue #328: [LANG-1238] Add overloaded methods to StringUtils w...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/328 Thank you for your patch. IMO, we should stop overloading our already giant StringUtils class with RegEx versions of methods. We already have some confusion (my op.) with some "replace*" methods taking a plain String and others taking RegEx Strings. Why not have a new RegExUtils class where all String input are RegEx? We can then deprecate the few StringUtil methods that do take RegEx Strings. Thoughts? ---
[GitHub] commons-lang issue #325: LANG-1392 methods for getting first non empty or no...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/325 Your proposal is fine, my suggestion currently falls in the YAGNI category. ---
[GitHub] commons-lang issue #325: LANG-1392 methods for getting first non empty or no...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/325 Thank you for your PR. It feels to me like this new API should be built on top of a more generate "getting Nth non empty or non blank value" ---
[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...
Github user garydgregory commented on the issue:
https://github.com/apache/commons-lang/pull/311
When I see code like
```
watch.visit(new StackWatch.TimingVisitor<String,String>() {
@Override
public void visitTiming(int level, List path,
StackWatch.Timing<String,String> node) {
assertTrue(node.getStopWatch().getNanoTime() >
stopWatch.getNanoTime());
}
});
```
I have my doubts as to this belonging in Commons Lang and not a new module
like a Commons Time (but nothing with Joda-Time or java.time overlap) or
Commons Perf.
This feels very 'framework-y' to me and beyond the otherwise simple Commons
Lang APIs.
Thoughts from anyone else?
---
[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/311 WRT "Try on the overall watch would have to behave differently, as it would both close the watch and stop the root timing..." I am not saying you would always use a try-with-resources, just that it should be an option. ---
[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...
Github user garydgregory commented on the issue:
https://github.com/apache/commons-lang/pull/311
I hope we do not advocate running code like the fragment you just quoted.
This is the same kind of issue we finally resolved in Apache Log4 2 where you
no longer need to write code like:
```
if (logger.isDebugEnabled()) {
logger.debug("This is " + foo + " and " +bar);
}
```
Instead you say:
`logger.debug("This is {} and {}", foo, bar);`
You can also delegate evaluation of expensive calls only if the logger is
enabled:
`logger.debug("This is {} and {}", () -> foo(), () -> bar());`
The logger is configured from a file or another code snippet.
IMO, we are already dangerously close to out of bounds of [lang] with the
current code, so I am mentioning this for exploration ATM. That said, I think
it would be good to see in text at least what a fully featured gadget looks
like before we include the current version in [lang]. Once in, it will likely
evolve, and then it might end up being moved to another component if it is no
longer 'lang-y'...
---
[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...
Github user garydgregory commented on the issue:
https://github.com/apache/commons-lang/pull/311
Hi All:
Have you considered allowing the timer to be used in a try-with-resources
statement such that
```
StackWatch<String,String> watch = new StackWatch<>("OuterFunction");
watch.start();
functionOne(watch);
watch.stop();
```
Becomes perhaps:
```
try (StackWatch<String,String> watch = new
StackWatch<>("OuterFunction").start()) {
functionOne(watch);
}
```
or more succinctly:
```
try (StackWatch<String,String> watch = StackWatch.start("OuterFunction")) {
functionOne(watch);
}
```
?
Gary
---
[GitHub] commons-lang issue #294: Added indexesOf method
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/294 The question here is whether to use the more grammatically correct "indices" or the more casual "indexes". ---
[GitHub] commons-lang pull request #273: add lastIndexOfAnyChar method just like inde...
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/273#discussion_r161278859
--- Diff: src/main/java/org/apache/commons/lang3/StringUtils.java ---
@@ -2122,6 +2122,81 @@ public static int indexOfAny(final CharSequence cs,
final String searchChars) {
return indexOfAny(cs, searchChars.toCharArray());
}
+/**
+ * Search a CharSequence to find the last index of any
--- End diff --
Use the active voice in Javadoc: "Returns...", "Searches...", "Gets..." and
so on.
---
[GitHub] commons-lang pull request #273: add lastIndexOfAnyChar method just like inde...
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/273#discussion_r161279018
--- Diff:
src/test/java/org/apache/commons/lang3/StringUtilsEqualsIndexOfTest.java ---
@@ -379,6 +379,44 @@ public void testIndexOfAny_StringCharArray() {
assertEquals(-1, StringUtils.indexOfAny("ab", 'z'));
}
+@Test
+public void testLastIndexOfAny_StringCharArray() {
+assertEquals(-1, StringUtils.lastIndexOfAnyChar(null, (char[])
null));
--- End diff --
Maybe use INDEX_NOT_FOUND as a static import instead of -1...
---
[GitHub] commons-lang pull request #273: add lastIndexOfAnyChar method just like inde...
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/273#discussion_r161278687
--- Diff: src/main/java/org/apache/commons/lang3/StringUtils.java ---
@@ -2122,6 +2122,81 @@ public static int indexOfAny(final CharSequence cs,
final String searchChars) {
return indexOfAny(cs, searchChars.toCharArray());
}
+/**
+ * Search a CharSequence to find the last index of any
+ * character in the given set of characters.
+ *
+ * A {@code null} String will return {@code -1}.
+ * A {@code null} search string will return {@code -1}.
+ *
+ *
+ * StringUtils.lastIndexOfAnyChar(null, *)= -1
+ * StringUtils.lastIndexOfAnyChar("", *) = -1
+ * StringUtils.lastIndexOfAnyChar(*, null)= -1
+ * StringUtils.lastIndexOfAnyChar(*, "") = -1
+ * StringUtils.lastIndexOfAnyChar("zzabyycdxx", "za") = 2
+ * StringUtils.lastIndexOfAnyChar("zzabyycdxx", "by") = 5
+ * StringUtils.lastIndexOfAnyChar("ab", 'a') = 0
+ * StringUtils.lastIndexOfAnyChar("ab", 'b') = 1
+ * StringUtils.lastIndexOfAnyChar("aba","z") = -1
+ *
+ *
+ * @param cs the CharSequence to check, may be null
+ * @param searchChars the chars to search for, may be null
+ * @return the last index of any of the chars, -1 if no match or null
input
+ */
+ public static int lastIndexOfAnyChar( final CharSequence cs,final
String searchChars) {
+return searchChars == null ? INDEX_NOT_FOUND :
lastIndexOfAnyChar(cs,searchChars.toCharArray());
+}
+
+ /**
+ * Search a CharSequence to find the last index of any
+ * character in the given set of characters.
+ *
+ * A {@code null} String will return {@code -1}.
+ * A {@code null} or zero length search array will return {@code
-1}.
+ *
+ *
+ * StringUtils.lastIndexOfAnyChar(null, *)= -1
+ * StringUtils.lastIndexOfAnyChar("", *) = -1
+ * StringUtils.lastIndexOfAnyChar(*, null)= -1
+ * StringUtils.lastIndexOfAnyChar(*, []) = -1
+ * StringUtils.lastIndexOfAnyChar("zzabyycdxx",['z','a']) = 2
+ * StringUtils.lastIndexOfAnyChar("zzabyycdxx",['b','y']) = 5
+ * StringUtils.lastIndexOfAnyChar("aba", ['z']) = -1
+ *
+ *
+ * @param cs the CharSequence to check, may be null
+ * @param searchChars the chars to search for, may be null
+ * @return the last index of any of the chars, -1 if no match or null
input
+ */
+public static int lastIndexOfAnyChar( final CharSequence cs,final
char... searchChars) {
--- End diff --
Fix formatting.
---
[GitHub] commons-lang pull request #278: Lang-1345 Enhance non-empty strings
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/278#discussion_r161277512
--- Diff: src/main/java/org/apache/commons/lang3/StringUtils.java ---
@@ -7429,6 +7429,55 @@ public static String defaultString(final String str,
final String defaultStr) {
return isEmpty(str) ? defaultStr : str;
}
+// Extensions
+
//---
+
+/**
+ * Returns either the passed in String with the specified prefix
and suffix attached,
+ * or if the String is whitespace, empty ("") or {@code null}, an
empty string.
+ *
+ * Whitespace is defined by {@link
Character#isWhitespace(char)}.
+ *
+ *
+ * StringUtils.extendIfNotBlank(null, "pre-", "-post") = ""
+ * StringUtils.extendIfNotBlank("", "pre-", "-post")= ""
+ * StringUtils.extendIfNotBlank(" ", "pre-", "-post") = ""
+ * StringUtils.extendIfNotBlank("bat", "pre-", "-post") =
"pre-bat-bost"
+ * StringUtils.extendIfNotBlank("bat", null, "-post") = "bat-post"
+ * StringUtils.extendIfNotBlank("bat", "pre-", null) = "pre-bat"
+ *
+ * @param str the String to check, may be null
+ * @param prefix the string to prepend if not blank. Null will be
converted to empty string.
+ * @param suffix the string to append if not blank. Null will be
converted to empty string.
+ * @return the passed in String with prefix and suffix added, or empty
string
+ * @see StringUtils#defaultString(String, String)
+ */
+public static String extendIfNotBlank(final String str, final String
prefix, final String suffix) {
+return isBlank(str) ? "" : defaultString(prefix) + str +
defaultString(suffix);
--- End diff --
Reuse the EMPTY constant.
---
[GitHub] commons-lang pull request #278: Lang-1345 Enhance non-empty strings
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/278#discussion_r161277531
--- Diff: src/main/java/org/apache/commons/lang3/StringUtils.java ---
@@ -7429,6 +7429,55 @@ public static String defaultString(final String str,
final String defaultStr) {
return isEmpty(str) ? defaultStr : str;
}
+// Extensions
+
//---
+
+/**
+ * Returns either the passed in String with the specified prefix
and suffix attached,
+ * or if the String is whitespace, empty ("") or {@code null}, an
empty string.
+ *
+ * Whitespace is defined by {@link
Character#isWhitespace(char)}.
+ *
+ *
+ * StringUtils.extendIfNotBlank(null, "pre-", "-post") = ""
+ * StringUtils.extendIfNotBlank("", "pre-", "-post")= ""
+ * StringUtils.extendIfNotBlank(" ", "pre-", "-post") = ""
+ * StringUtils.extendIfNotBlank("bat", "pre-", "-post") =
"pre-bat-bost"
+ * StringUtils.extendIfNotBlank("bat", null, "-post") = "bat-post"
+ * StringUtils.extendIfNotBlank("bat", "pre-", null) = "pre-bat"
+ *
+ * @param str the String to check, may be null
+ * @param prefix the string to prepend if not blank. Null will be
converted to empty string.
+ * @param suffix the string to append if not blank. Null will be
converted to empty string.
+ * @return the passed in String with prefix and suffix added, or empty
string
+ * @see StringUtils#defaultString(String, String)
+ */
+public static String extendIfNotBlank(final String str, final String
prefix, final String suffix) {
+return isBlank(str) ? "" : defaultString(prefix) + str +
defaultString(suffix);
+}
+
+/**
+ * Returns either the passed in String with the specified prefix
and suffix attached,
+ * or if the String is empty ("") or {@code null}, an empty string.
+ *
+ *
+ * StringUtils.extendIfNotEmpty(null, "pre-", "-post") = ""
+ * StringUtils.extendIfNotEmpty("", "pre-", "-post")= ""
+ * StringUtils.extendIfNotEmpty(" ", "pre-", "-post") = "pre- -post"
+ * StringUtils.extendIfNotEmpty("bat", "pre-", "-post") =
"pre-bat-bost"
+ * StringUtils.extendIfNotEmpty("bat", null, "-post") = "bat-post"
+ * StringUtils.extendIfNotEmpty("bat", "pre-", null) = "pre-bat"
+ *
+ * @param str the String to check, may be null
+ * @param prefix the string to prepend if not empty. Null will be
converted to empty string.
+ * @param suffix the string to append if not empty. Null will be
converted to empty string.
+ * @return the passed in String with prefix and suffix added, or empty
string
+ * @see StringUtils#defaultString(String, String)
+ */
+public static String extendIfNotEmpty(final String str, final String
prefix, final String suffix) {
+return isEmpty(str) ? "" : defaultString(prefix) + str +
defaultString(suffix);
--- End diff --
Reuse the EMPTY constant.
---
[GitHub] commons-lang issue #304: Fix DateUtilsTest to work reliably on Java 9
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/304 The current git master build OK for me with 'mvn clean verify' and Oracle Java 9.0.1. ---
[GitHub] commons-lang pull request #296: LANG-1355: Add FastTimeZone to decrease Time...
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/296#discussion_r143804862
--- Diff: src/main/java/org/apache/commons/lang3/time/FastTimeZone.java ---
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3.time;
+
+import java.util.TimeZone;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Faster methods to produce custom time zones.
+ *
+ * @since 3.7
+ */
+public class FastTimeZone {
+
+private static final TimeZone GREENWICH = new GmtTimeZone(false, 0, 0);
+
+// do not instantiate
+private FastTimeZone() {
+}
+
+/**
+ * Get the GMT TimeZone.
+ * @return A TimeZone with a raw offset of zero.
+ */
+public static TimeZone getGmtTimeZone() {
+return GREENWICH;
+}
+
+/**
+ * Get a TimeZone, looking first for GMT custom ids, then falling back
to Olson ids.
--- End diff --
In future patches, you can use the active voice for Javadocs: "Gets a
TimeZone..." instead of "Get...". :-)
---
[GitHub] commons-lang pull request #:
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/commit/d848328a7aa54f73117b8f13ce6e67049dc9502e#commitcomment-24866944 "3.6 already requires java 7." I think it is nice to state the plaform requirement for any component. Often time, one is left guessing or hunting down POMs for compiler settings. ---
[GitHub] commons-lang issue #297: Add a rule of Locale.ENGLISH to String.toUpperCase(...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/297 I fixed some tests and code main code here and there and I think all the changes we need are in. If you feel otherwise, please update this PR based on git master. Thank you! ---
[GitHub] commons-lang issue #297: Add a rule of Locale.ENGLISH to String.toUpperCase(...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/297 I am splitting out the FastDateParser changes here: https://issues.apache.org/jira/browse/LANG-1357 ---
[GitHub] commons-lang issue #297: Add a rule of Locale.ENGLISH to String.toUpperCase(...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/297 Shouldn't this be done with Locale.ROOT? ---
[GitHub] commons-lang issue #290: Added string methods
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/290 The anagram method belongs in commons-text IMO. ---
[GitHub] commons-lang issue #282: SwappedPair constructed as Pair.of(rhs,lhs)
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/282 +1 on the unit test request, we need it to avoid regressions. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #277: run maven defaultGoal from Travis
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/277 Patch applied. Thank you! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #277: run maven defaultGoal from Travis
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/277 This PR has no description. Can you please specify why we want this change? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #:
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/commit/e4c72a5522aabfa6a660088aa9262d849756e464#commitcomment-19566492 In src/changes/changes.xml: In src/changes/changes.xml on line 48: Pretty soon I'll get a color TV as well ;-) On Oct 23, 2016 11:32 AM, "Pascal Schumacher" <notificati...@github.com> wrote: > pretty optimistic ;) :) > > â > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/apache/commons-lang/commit/e4c72a5522aabfa6a660088aa9262d849756e464#commitcomment-19532086>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABIfN5C9m3W-Epmenh4PbL54hoAh1Z8Yks5q26g7gaJpZM4KeNKQ> > . > --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #170: Added RandomUtils#nextBoolean() method
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/170 Created https://issues.apache.org/jira/browse/LANG-1253 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #171: Removing test redundant for org.apache.commons.lang...
Github user garydgregory commented on the issue: https://github.com/apache/commons-lang/pull/171 I agree, -1 to remove the test. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: LANG-781 Added methods to ObjectUtils c...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/108#issuecomment-219860202 There is a discussion going on right now on the dev mailing list about for-each vs. for. Let's see how that plays out. See https://commons.apache.org/mail-lists.html --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: Adding unwrap and unwrapFull methods to...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/136#issuecomment-218867766 The name unwrapFull sounds weird to me. Perhaps unwrapAll? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: LANG-1184: StringUtils#normalizeSpace n...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/113#issuecomment-217737900 Yes, let's add a new method. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: LANG-781 Added methods to ObjectUtils c...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/108#issuecomment-209650500 Yes: - Thank you for the patch. - The Javadoc has some bogus text, like starting with "". You do not need that. - Do not use a for-each loop to traverse an array. This creates an iterator when none is needed. Use a normal for loop, which will not create the extra Iterator to GC. This matters when a libraries is used on the critical path and/or environments trying to do low/no GC. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: Fix indexOfIgnoreCase javadoc and provi...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/115#issuecomment-209649478 Patch applied. Thank you. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: Formatting ArrayUtils
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/128#issuecomment-209648589 Patch applied. Thank you. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: Fix typo on appendIfMissing javadoc
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/129#issuecomment-209647548 Looks like this has been fixed. This issue can be closed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: General cleanup
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/127#issuecomment-209628290 It is what it is for whatever reason the original author intended, that person must have thought it important for subclasses to NOT be able to hide the methods. The class itself should probably be final like any utility class, but we cannot change that in a minor release since it would break BC. To cut to the chase: I am OK with apply the patch if it does not include the 'final' changes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: General cleanup
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/127#issuecomment-209622299 Final methods is a bigger discussion than what this ticket suggests. It's NOT a code style clean up. For a general discussion on final, but not specifically of final methods, please see https://garygregory.wordpress.com/2013/01/26/the-final-kiss-in-java/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: General cleanup
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/127#issuecomment-209620015 @pivovarit: One more thing: Please do not remove 'final' from anything. Send me another diff and I'll review again. Thank you, Gary --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: Fix typo in API docs
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/131#issuecomment-209610697 Thank you for the report. Fixed in Git master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: LANG-1146 Add helper method to SystemUt...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/103#issuecomment-195886347 See Git master for an alternate solution. You can close https://issues.apache.org/jira/browse/LANG-1146 and this issue if you like the solution and/or can test on z/OS. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: Fix typo on appendIfMissing javadoc
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/129#issuecomment-194417825 Thank you for the PR. In Git master now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: LANG-1146 Add helper method to SystemUt...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/103#issuecomment-192982662 Here are some sys props from one of our z/OS systems: Start system properties (71): awt.toolkit = sun.awt.X11.XToolkit com.ibm.cpu.endian = big com.ibm.jcl.checkClassPath = com.ibm.oti.configuration = scar com.ibm.oti.jcl.build = 20150630_255633 com.ibm.oti.shared.enabled = false com.ibm.oti.vm.bootstrap.library.path = /rsusr/java/IBM/J8.0_64/lib/s390x/compressedrefs:/rsusr/java/IBM/J8.0_64/lib/s390x com.ibm.oti.vm.library.version = 28 com.ibm.packed.version = 2 com.ibm.system.agent.path = /rsusr/java/IBM/J8.0_64/lib/s390x com.ibm.util.extralibs.properties = com.ibm.vm.bitmode = 64 com.ibm.zero.version = 2 file.encoding = IBM-1047 file.encoding.pkg = sun.io file.separator = / ibm.signalhandling.rs = false ibm.signalhandling.sigchain = false ibm.signalhandling.sigint = true ibm.system.encoding = IBM-1047 java.awt.fonts = java.awt.graphicsenv = sun.awt.X11GraphicsEnvironment java.awt.printerjob = sun.print.PSPrinterJob java.class.path = *** java.class.version = 52.0 java.compiler = j9jit28 java.endorsed.dirs = /rsusr/java/IBM/J8.0_64/lib/endorsed java.ext.dirs = /rsusr/java/IBM/J8.0_64/lib/ext java.fullversion = JRE 1.8.0 IBM J9 2.8 z/OS s390x-64 Compressed References 20150630_255633 (JIT enabled, AOT enabled) J9VM - R28_jvm.28_20150630_1742_B255633 JIT - tr.r14.java_20150625_95081.01 GC - R28_jvm.28_20150630_1742_B255633_CMPRSS J9CL - 20150630_255633 java.home = /rsusr/java/IBM/J8.0_64 java.io.tmpdir = /tmp java.library.path = *** java.runtime.name = Java(TM) SE Runtime Environment java.runtime.version = pmz6480sr1fp10-20150716_01 (SR1 FP10) java.specification.name = Java Platform API Specification java.specification.vendor = Oracle Corporation java.specification.version = 1.8 java.util.prefs.PreferencesFactory = java.util.prefs.FileSystemPreferencesFactory java.vendor = IBM Corporation java.vendor.url = http://www.ibm.com/ java.version = 1.8.0 java.vm.info = JRE 1.8.0 z/OS s390x-64 Compressed References 20150630_255633 (JIT enabled, AOT enabled) J9VM - R28_jvm.28_20150630_1742_B255633 JIT - tr.r14.java_20150625_95081.01 GC - R28_jvm.28_20150630_1742_B255633_CMPRSS J9CL - 20150630_255633 java.vm.name = IBM J9 VM java.vm.specification.name = Java Virtual Machine Specification java.vm.specification.vendor = Oracle Corporation java.vm.specification.version = 1.8 java.vm.vendor = IBM Corporation java.vm.version = 2.8 line.separator = 0x15 os.arch = s390x os.encoding = ISO8859_1 os.name = z/OS os.version = 02.02.00 path.separator = : platform.notASCII = true sun.arch.data.model = 64 sun.boot.class.path = *** sun.boot.library.path = *** sun.cpu.endian = big sun.io.unicode.encoding = UnicodeBig sun.java.command = *** sun.java.launcher = SUN_STANDARD sun.jnu.encoding = IBM-1047 sun.security.policy.utf8 = false user.country = US user.dir = *** user.home = *** user.language = en user.name = *** user.timezone = GMT-05:00 user.variant = --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: LANG-1146 Add helper method to SystemUt...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/103#issuecomment-192962652 Can we come up a check that works on all version of z/OS? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: General cleanup
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/127#issuecomment-191189556 Ok, I'll look tomorrow. In the meantime, can you provide a diff file for all of this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: Fix for incorrect comment on StringUtil...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/125#issuecomment-191134916 Patch applied, thank you! Commit: cd63fed74cbf264f9ae48b00700f143211a4bcd0 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: General cleanup
Github user garydgregory commented on the pull request:
https://github.com/apache/commons-lang/pull/127#issuecomment-191131937
Sadly, this kind of change is not a "Good Thing":
@@ -7266,14 +7245,14 @@ public static int indexOfDifference(final
CharSequence... css) {
// find the min and max string lengths; this avoids checking to
make
// sure we are not exceeding the length of the string each time
through
// the bottom loop.
-for (int i = 0; i < arrayLen; i++) {
-if (css[i] == null) {
+for (CharSequence cs : css) {
+if (cs == null) {
anyStringNull = true;
shortestStrLen = 0;
} else {
allStringsNull = false;
-shortestStrLen = Math.min(css[i].length(), shortestStrLen);
-longestStrLen = Math.max(css[i].length(), longestStrLen);
+shortestStrLen = Math.min(cs.length(), shortestStrLen);
+longestStrLen = Math.max(cs.length(), longestStrLen);
}
}
That is because the compiler converts use an Iterator to traverse an array
with a for-each loop which is slower than traversing it with index access. Rule
of thumb: don't use for-each over an array when performance matters. In
low-level libraries like Commons, performance usually matters.
Please review your patch and remove such changes.
Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang pull request: LANG-1210 Fixed JavaDoc
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/126#issuecomment-190505830 Fixed with https://issues.apache.org/jira/browse/LANG-1211 and commit d1a3255600da34f4b69dc082c4441ae140452fee --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: General cleanup
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/127#issuecomment-190406183 I like to look at patch files, which ever way produced. ;-) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: General cleanup
Github user garydgregory commented on the pull request:
https://github.com/apache/commons-lang/pull/127#issuecomment-190350273
Hello,
Thank you for your interest in Apache Commons Lang.
Changes like this are not clean ups because it will cause an unboxing
compiler warning.
@@ -465,7 +465,7 @@ public static int toInteger(final Boolean bool, final
int trueValue, final int f
if (bool == null) {
return nullValue;
}
-return bool.booleanValue() ? trueValue : falseValue;
+return bool ? trueValue : falseValue;
}
Would you be willing to update your patch?
Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang pull request: LANG-1184: StringUtils#normalizeSpace n...
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/113#issuecomment-172369835 Hi All, I'm more concerned about what the proper behavior of the method is, rather than its behavior in some past arbitrary version. Since Java does not define a nbsp as a whitespace, it should not be normalized away IMO. Now, if you want it normalized, we could talk about adding another method or documenting how to deal with this special use case. Are there other characters that are ws-like characters that return false for Character.isWhitespace(). Unicode is pretty rich, maybe there is more. What would this new method be called? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request: LANG-1184: StringUtils#normalizeSpace n...
Github user garydgregory commented on the pull request:
https://github.com/apache/commons-lang/pull/113#issuecomment-172291572
Thank you for your interest in [lang].
Java says a non-breaking whitespace is not a whitespace:
```java
Character.isWhitespace('\u00A0'));
```
returns `false`.
So I am not sure this patch is valid.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang pull request: modify note at line 1230
Github user garydgregory commented on the pull request: https://github.com/apache/commons-lang/pull/120#issuecomment-171797905 In Git master, please verify and close. Offically tracked here: https://issues.apache.org/jira/browse/LANG-1200 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
