Skip to content

handle the {index} parameter like a native MessageFormat argument. #969

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

melchisedek
Copy link

For example that allows to specified leading zero by using
{index,number,0000}

For example that allows to specified leading zero by using
{index,number,0000}
String finalPattern = pattern.replaceAll("\\{index\\}",
Integer.toString(index));
String finalPattern = pattern;
Pattern indexMatcherPattern = Pattern.compile("(\\{)index([^\\}]*\\})");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you extract a constant for this Pattern?

@kcooney
Copy link
Member

kcooney commented Jul 31, 2014

Please add some tests for this.

Pattern indexMatcherPattern = Pattern.compile("(\\{)index([^\\}]*\\})");
Matcher matcher = indexMatcherPattern.matcher(pattern);
if (matcher.find()) {
String idxPattern = matcher.group(1) + 0 + matcher.group(2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use "0" instead of 0, because it is clearer.

melchisedek added 3 commits July 31, 2014 23:53
* extract a constant for this Pattern
* use while loop (replace and not replaceall)
* use "0" instead of 0
@@ -163,6 +165,8 @@
* @since 4.0
*/
public class Parameterized extends Suite {
private static final String INDEX_MATCHER_PATTERN = "(\\{)index([^\\}]*\\})";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant create a constant for the Pattern. Compiling a pattern isn't cheap

@avandeursen
Copy link
Contributor

Looks like an interesting addition.

This also requires adjusting the documentation.

I would suggest adding an extra paragraph (after line 75 in the changed file) giving a number of insightful and compelling examples on how to make good use of this new feature.

@adrianosimoes
Copy link

Should the conflicts be fixed, or maybe it's too late and we should just close this pull request?

@marcphilipp marcphilipp changed the base branch from master to main June 21, 2020 17:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants