Skip to content

Add transformTagName option to transform tag names when parsing#469

Merged
amitguptagwl merged 1 commit intoNaturalIntelligence:masterfrom
feederco:lowercase-tag-names
Jul 6, 2022
Merged

Add transformTagName option to transform tag names when parsing#469
amitguptagwl merged 1 commit intoNaturalIntelligence:masterfrom
feederco:lowercase-tag-names

Conversation

@erkie
Copy link
Contributor

@erkie erkie commented Jun 29, 2022 •
edited
Loading

Purpose / Goal

Add an option to transform tag names in output of .parse. Option name is:

transformTagName: false | (tagName: string) => string;

Pasting the test here because it's a great sample of what this PR does:

const xmlData = `<?xml version='1.0'?>
  <root>
    <person>Person 1</person>
    <Person>Person 2</Person>
    <PERSON>Person 3</PERSON>
    <person>Person 4</person>
  </root>
`;

const parser = new XMLParser({
    ignoreDeclaration: true,
    transformTagName: (tagName) => tagName.toLowerCase()
});
let result = parser.parse(xmlData);

const expected = {
    "root": {
        "person": [
            "Person 1",
            "Person 2",
            "Person 3",
            "Person 4"
        ]
    }
};

expect(result).toEqual(expected);```

# Type
Please mention the type of PR
<!-- choose one by changing [ ] to [x] -->
* [ ]Bug Fix
* [ ]Refactoring / Technology upgrade
* [x]New Feature

@amitguptagwl
Copy link
Member

amitguptagwl commented Jul 1, 2022

Thanks for this PR @erkie . I'm just wondering that there can be multiple options to format the tag or attribute name. And probably everyone need some combination as per their need. So it could be better to create a function that can be overridden by the user to apply particular formatting.

In this way, some more requirements can be covered eg replacing hyphen with camel casing etc.

@erkie erkie force-pushed the lowercase-tag-names branch from 344eade to 9ee8d4b Compare July 1, 2022 13:01
@erkie erkie changed the title Add lowerCaseTagNames option to lowerCaseTagNames when parsing Add transformTagNames option to transform tag names when parsing Jul 1, 2022
@erkie
Copy link
Contributor Author

erkie commented Jul 1, 2022

Thanks for the feedback @amitguptagwl I totally agree with you. I amended the PR to be transformTagNames instead so it's 100% customizable now.

@amitguptagwl
Copy link
Member

amitguptagwl commented Jul 4, 2022

It looks like some tests are failing. Can you please check?

 Failures:
1) XMLParser should parse lowercase tagnames
  Message:
    Expected object not to have properties
        ?xml: ''
  Stack:
    Error: Expected object not to have properties
        ?xml: ''
        at <Jasmine>
        at UserContext.<anonymous> (/home/runner/work/fast-xml-parser/fast-xml-parser/spec/transform_tagname_spec.js:9:374)
        at <Jasmine>

@erkie erkie force-pushed the lowercase-tag-names branch from 9ee8d4b to 5e38a80 Compare July 5, 2022 19:15
@erkie
Copy link
Contributor Author

erkie commented Jul 5, 2022

@amitguptagwl thanks for that, it was an error with my tests. It has been fixed now.

Also, I changed the option name from transformTagNames to transformTagName since it made more sense.

@erkie erkie changed the title Add transformTagNames option to transform tag names when parsing Add transformTagName option to transform tag names when parsing Jul 5, 2022
@coveralls
Copy link

coveralls commented Jul 6, 2022

Coverage Status

Coverage decreased (-1.3%) to 98.544% when pulling 5e38a80 on feederco:lowercase-tag-names into 2203711 on NaturalIntelligence:master.

@coveralls
Copy link

coveralls commented Jul 6, 2022 •
edited
Loading

Coverage Status

Coverage decreased (-1.3%) to 98.544% when pulling 5e38a80 on feederco:lowercase-tag-names into 2203711 on NaturalIntelligence:master.

@amitguptagwl amitguptagwl merged commit 6ad40ee into NaturalIntelligence:master Jul 6, 2022
@erkie erkie deleted the lowercase-tag-names branch July 6, 2022 07:15
@erkie erkie mentioned this pull request Jul 6, 2022
Closed
4 tasks
@erkie erkie mentioned this pull request Dec 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@erkie @amitguptagwl @coveralls