Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix: keep quotes in the case of keys that are octal literals #4309

Merged
merged 3 commits into from
Mar 20, 2023

Conversation

nissy-dev
Copy link
Contributor

@nissy-dev nissy-dev commented Mar 18, 2023

Summary

Fix #4284

Test Plan

I updated tests and confirmed the issue was fixed.

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Mar 18, 2023

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 10b568f
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/641879e67213720008fe9afe

@github-actions github-actions bot added the A-Formatter Area: formatter label Mar 18, 2023
@@ -247,6 +247,11 @@ impl<'token> LiteralStringNormaliser<'token> {
let mut has_seen_number = false;
text_to_check.chars().enumerate().all(|(index, c)| {
if index == 0 && c.is_numeric() {
Copy link
Contributor

@denbezrukov denbezrukov Mar 18, 2023

Choose a reason for hiding this comment

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

I'm wondering if we have to use here is_ascii_digit What do you think?
Because is_numeric https://doc.rust-lang.org/std/primitive.char.html#method.is_numeric

assert!('৬'.is_numeric());
assert!('¾'.is_numeric());
assert!('①'.is_numeric());

I'm not sure if it's valid.
Could you test please? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

@denbezrukov do you have an use case/test case that we can use ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use is_ascii_digit and update test 👍


const x = {
¾¾¾¾: "test1",
①: "test2",
Copy link
Contributor

@denbezrukov denbezrukov Mar 19, 2023

Choose a reason for hiding this comment

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

I guess that correct one is

const x = {
	"¾¾¾¾": "test1",
	"①": "test2",
};

Copy link
Contributor Author

@nissy-dev nissy-dev Mar 20, 2023

Choose a reason for hiding this comment

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

I tried to fix in 10b568f.

@Conaclos Conaclos added this to the v12.0.0 milestone Mar 19, 2023
@ematipico ematipico merged commit 8274551 into rome:main Mar 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Formatter strips quotes from keys that are octal literals
4 participants