You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by twogee <gi...@git.apache.org> on 2018/11/06 21:32:11 UTC
[GitHub] ant pull request #78: A new CharSet type to hold available Charset names
GitHub user twogee opened a pull request:
https://github.com/apache/ant/pull/78
A new CharSet type to hold available Charset names
I believe that might be useful when validating "encoding" (or "charset") attributes
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/twogee/ant charset-type
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/ant/pull/78.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #78
----
commit 033d68c7f6ecf382f84eec5ab0c2bb91eb9bd6fb
Author: twogee <g....@...>
Date: 2018-11-06T21:27:55Z
A new CharSet type to hold available Charset names
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/ant/pull/78
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant%20Github-PR-Windows/90/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the issue:
https://github.com/apache/ant/pull/78
Both files need license headers. Other than that, looks good to me.
You envision this type to be used as argument type in `setEncoding` overloads?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/ant/pull/78
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant%20Github-PR-Windows/95/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by twogee <gi...@git.apache.org>.
Github user twogee commented on the issue:
https://github.com/apache/ant/pull/78
License headers added. Yes, it should be used with `setEncoding` (or in some tasks `setCharset`). Now that I think about it image/imageio tasks have an "encoding" attribute which is a misnomer. I'd like to deprecate it in imageio and use a "format" instead (with a proper Enumerated Attribute to boot 😁).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/ant/pull/78
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant%20Github-PR-Windows/92/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant pull request #78: A new CharSet type to hold available Charset names
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on a diff in the pull request:
https://github.com/apache/ant/pull/78#discussion_r232673446
--- Diff: src/tests/junit/org/apache/tools/ant/types/CharSetTest.java ---
@@ -0,0 +1,20 @@
+package org.apache.tools.ant.types;
+
+import org.apache.tools.ant.BuildException;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+public class CharSetTest {
+ @Test
+ public void testCorrectNames() {
+ String[] expected = {"UTF-8", "ISO-8859-1", "037", "us", "IBM500"};
+ Arrays.stream(expected).forEach(new CharSet()::setValue);
+ }
+
+ @Test(expected = BuildException.class)
+ public void testNonExistentNames() {
+ String[] nonexistent = {"mojibake", "dummy"};
+ Arrays.stream(nonexistent).forEach(new CharSet()::setValue);
--- End diff --
is the test for "dummy" ever going to be run?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the issue:
https://github.com/apache/ant/pull/78
looks good.
The imageio tasks haven't been part of any release, yet, no reason to deprecate methods, just change them before we cut the next release IMHO.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant pull request #78: A new CharSet type to hold available Charset names
Posted by twogee <gi...@git.apache.org>.
Github user twogee closed the pull request at:
https://github.com/apache/ant/pull/78
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/ant/pull/78
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant%20Github-PR-Linux/86/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant issue #78: A new CharSet type to hold available Charset names
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/ant/pull/78
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant%20Github-PR-Linux/84/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant pull request #78: A new CharSet type to hold available Charset names
Posted by twogee <gi...@git.apache.org>.
Github user twogee commented on a diff in the pull request:
https://github.com/apache/ant/pull/78#discussion_r232795363
--- Diff: src/tests/junit/org/apache/tools/ant/types/CharSetTest.java ---
@@ -0,0 +1,20 @@
+package org.apache.tools.ant.types;
+
+import org.apache.tools.ant.BuildException;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+public class CharSetTest {
+ @Test
+ public void testCorrectNames() {
+ String[] expected = {"UTF-8", "ISO-8859-1", "037", "us", "IBM500"};
+ Arrays.stream(expected).forEach(new CharSet()::setValue);
+ }
+
+ @Test(expected = BuildException.class)
+ public void testNonExistentNames() {
+ String[] nonexistent = {"mojibake", "dummy"};
+ Arrays.stream(nonexistent).forEach(new CharSet()::setValue);
--- End diff --
I was lazy... 😁 the tests are properly parameterised now
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org