[GitHub] [maven] michael-o commented on a diff in pull request #1098: [MNG-7774] Maven config and command line interpolation
michael-o commented on code in PR #1098:
URL: https://github.com/apache/maven/pull/1098#discussion_r1185941475
##
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##
@@ -364,6 +369,38 @@ public void testVersionStringWithoutAnsi() throws
Exception {
assertEquals(MessageUtils.stripAnsiCodes(versionOut), versionOut);
}
+@Test
+public void testPropertiesInterpolation() throws Exception {
+// Arrange
+CliRequest request = new CliRequest(
+new String[] {
+"-Dfoo=bar",
+"-DvalFound=s${foo}i",
+"-DvalNotFound=s${foz}i",
+"-DvalRootDirectory=${session.rootDirectory}/.mvn/foo",
+"-DvalTopDirectory=${session.topDirectory}/pom.xml",
Review Comment:
This I understand now. Hope they won't be misunderstood by users.
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #1098: [MNG-7774] Maven config and command line interpolation
michael-o commented on code in PR #1098:
URL: https://github.com/apache/maven/pull/1098#discussion_r1185835588
##
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##
@@ -364,6 +369,38 @@ public void testVersionStringWithoutAnsi() throws
Exception {
assertEquals(MessageUtils.stripAnsiCodes(versionOut), versionOut);
}
+@Test
+public void testPropertiesInterpolation() throws Exception {
+// Arrange
+CliRequest request = new CliRequest(
+new String[] {
+"-Dfoo=bar",
+"-DvalFound=s${foo}i",
+"-DvalNotFound=s${foz}i",
+"-DvalRootDirectory=${session.rootDirectory}/.mvn/foo",
+"-DvalTopDirectory=${session.topDirectory}/pom.xml",
Review Comment:
I have a problem with these two expressions: They haven't been back ported
to 3.9.x, no? So they are fake here and not work throughout the entire build?
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #1098: [MNG-7774] Maven config and command line interpolation (take two)
michael-o commented on code in PR #1098:
URL: https://github.com/apache/maven/pull/1098#discussion_r1183379283
##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -526,8 +578,39 @@ private void commands(CliRequest cliRequest) {
// Needed to make this method package visible to make writing a unit test
possible
// Maybe it's better to move some of those methods to separate class (SoC).
-void properties(CliRequest cliRequest) {
-populateProperties(cliRequest.commandLine,
cliRequest.systemProperties, cliRequest.userProperties);
+void properties(CliRequest cliRequest) throws ExitException {
+try {
+populateProperties(cliRequest, cliRequest.systemProperties,
cliRequest.userProperties);
+
+StringSearchInterpolator interpolator =
Review Comment:
I wonder they there are two what the difference is.
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #1098: [MNG-7774] Maven config and command line interpolation (take two)
michael-o commented on code in PR #1098:
URL: https://github.com/apache/maven/pull/1098#discussion_r1183348912
##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -526,8 +578,39 @@ private void commands(CliRequest cliRequest) {
// Needed to make this method package visible to make writing a unit test
possible
// Maybe it's better to move some of those methods to separate class (SoC).
-void properties(CliRequest cliRequest) {
-populateProperties(cliRequest.commandLine,
cliRequest.systemProperties, cliRequest.userProperties);
+void properties(CliRequest cliRequest) throws ExitException {
+try {
+populateProperties(cliRequest, cliRequest.systemProperties,
cliRequest.userProperties);
+
+StringSearchInterpolator interpolator =
Review Comment:
I remember that one has superseded the other, I don't exactly remember.
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #1098: [MNG-7774] Maven config and command line interpolation (take two)
michael-o commented on code in PR #1098:
URL: https://github.com/apache/maven/pull/1098#discussion_r1183348436
##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -140,9 +146,14 @@ public class MavenCli {
private static final String EXT_CLASS_PATH = "maven.ext.class.path";
-private static final String EXTENSIONS_FILENAME = ".mvn/extensions.xml";
+private static final String DOT_MVN = ".mvn";
-private static final String MVN_MAVEN_CONFIG = ".mvn/maven.config";
+private static final String UNABLE_TO_FIND_ROOT_PROJECT_MESSAGE = "Unable
to find the root directory. Create a "
++ DOT_MVN + " directory in the project root directory to identify
it.";
+
+private static final String EXTENSIONS_FILENAME = DOT_MVN +
"/extensions.xml";
Review Comment:
Shit, you are right. It just popped my eye.
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #1098: [MNG-7774] Maven config and command line interpolation (take two)
michael-o commented on code in PR #1098:
URL: https://github.com/apache/maven/pull/1098#discussion_r1183323362
##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -140,9 +146,14 @@ public class MavenCli {
private static final String EXT_CLASS_PATH = "maven.ext.class.path";
-private static final String EXTENSIONS_FILENAME = ".mvn/extensions.xml";
+private static final String DOT_MVN = ".mvn";
-private static final String MVN_MAVEN_CONFIG = ".mvn/maven.config";
+private static final String UNABLE_TO_FIND_ROOT_PROJECT_MESSAGE = "Unable
to find the root directory. Create a "
++ DOT_MVN + " directory in the project root directory to identify
it.";
+
+private static final String EXTENSIONS_FILENAME = DOT_MVN +
"/extensions.xml";
+
+private static final String MVN_MAVEN_CONFIG = DOT_MVN + "/maven.config";
Review Comment:
Suffers from the same more or less.
##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -140,9 +146,14 @@ public class MavenCli {
private static final String EXT_CLASS_PATH = "maven.ext.class.path";
-private static final String EXTENSIONS_FILENAME = ".mvn/extensions.xml";
+private static final String DOT_MVN = ".mvn";
-private static final String MVN_MAVEN_CONFIG = ".mvn/maven.config";
+private static final String UNABLE_TO_FIND_ROOT_PROJECT_MESSAGE = "Unable
to find the root directory. Create a "
++ DOT_MVN + " directory in the project root directory to identify
it.";
+
+private static final String EXTENSIONS_FILENAME = DOT_MVN +
"/extensions.xml";
Review Comment:
That's not a filename anymore, but a path.
##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -526,8 +578,39 @@ private void commands(CliRequest cliRequest) {
// Needed to make this method package visible to make writing a unit test
possible
// Maybe it's better to move some of those methods to separate class (SoC).
-void properties(CliRequest cliRequest) {
-populateProperties(cliRequest.commandLine,
cliRequest.systemProperties, cliRequest.userProperties);
+void properties(CliRequest cliRequest) throws ExitException {
+try {
+populateProperties(cliRequest, cliRequest.systemProperties,
cliRequest.userProperties);
+
+StringSearchInterpolator interpolator =
Review Comment:
Any reason not to use the `StringRegexInterpolator`?
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
