Skip to content

Fix Ids in FreeMarker templates for values greater than 999#413

Merged
anshooarora merged 1 commit intoextent-framework:masterfrom
N0r1uno:master
Jun 19, 2024
Merged

Fix Ids in FreeMarker templates for values greater than 999#413
anshooarora merged 1 commit intoextent-framework:masterfrom
N0r1uno:master

Conversation

@N0r1uno
Copy link
Contributor

@N0r1uno N0r1uno commented Oct 15, 2023

Depending on locale setting, current conversion of ids can result in point or comma separated numbers.
This breaks things like json code blocks because of invalid JS function naming (e.g. jsonTreeCreate1.000())

Behaviour can be reproduced by the follwing code:

for (int i = 0; i < 1000; i++)
  MarkupHelper.createCodeBlock("{ 'key': 'value' }", CodeLanguage.JSON).getMarkup();
System.out.println(MarkupHelper.createCodeBlock("{ 'key': 'value' }", CodeLanguage.JSON).getMarkup());

Fixed by setting formatting options in affected .ftl files

@apex-sewilliams
Copy link

I am running into this too. It looks like there was an issue with the build but it does not look like it's with anything you touched. Are you able to rerun the build to see if it passes now?

@N0r1uno
Copy link
Contributor Author

N0r1uno commented Jan 6, 2024

Yep, that failure looks unrelated. I think I am not able to do anything here.
@anshooarora could you maybe take a look at it again?

@apex-sewilliams
Copy link

@N0r1uno are you able to close the PR and open a new one? Maybe that will unstick something?

@SebastianRG2
Copy link

Does this change have any follow-up or any other PR? Can we move forward with this or is there an alternative solution?

What is the stop of this change?

@anshooarora
Copy link
Member

I'm not quite sure why this build failed and I see no errors generating this template. Merging.

@anshooarora anshooarora merged commit b665ead into extent-framework:master Jun 19, 2024
@SebastianRG2
Copy link

Hi, its possible to create a new release where the ‘Fix Ids in FreeMarker templates for values greater than 999’ is included, so that we can consume the library with this fix?

@anshooarora

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants