Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add save extra namespaces #18

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

whatever098
Copy link

Added the saveExtraNamespaces input parameter and modified the top-level element to write to disk to ensure that the top-level element can write extraNamespaces

s_zhangziang added 3 commits June 24, 2024 17:09
Added the saveExtraNamespaces input parameter and modified the top-level element to write to disk to ensure that the top-level element can write extraNamespaces
@pjfanning
Copy link
Contributor

Could you add a unit test?

@whatever098
Copy link
Author

Could you add a unit test?

OK,I will do this

@pjfanning
Copy link
Contributor

@whatever098 could you give some background on why adding the unused extra namespaces is useful? I'm struggling to see how this is useful.

@whatever098
Copy link
Author

@pjfanning When we save the java code generated by xsd as xml, we need this method to set the namespace

@pjfanning
Copy link
Contributor

XMLBeans adds the namespace declarations that it needs. I'm against merging this. What if someone adds an 'extra namespace' with the same prefix as one that XMLBeans generates for its own needs? We've gone 20 years without needing this support.

Provide a very detailed example of why you need this - something that fails without this support - or I will remain against adding this.

You are free to fork XMLBeans and add whatever you like to your fork.

assertEquals("http://www.nits.org.cn/uof3/zh-cn/2021/graph", root.getAttribute("xmlns:图"));
assertEquals("http://www.nits.org.cn/uof3/zh-cn/2021/wordproc", root.getAttribute("xmlns:字"));
assertEquals("http://www.nits.org.cn/uof3/zh-cn/2021/spreadsheet", root.getAttribute("xmlns:表"));
assertEquals("http://www.nits.org.cn/uof3/zh-cn/2021/presentation", root.getAttribute("xmlns:演"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a single one of these namespaces is used in the XML. I am -1 to merging this. I see no point in supporting adding unused namespace declarations.

@whatever098
Copy link
Author

whatever098 commented Jul 2, 2024

@pjfanning Let me explain the reason in detail. When I generate a jar with an xsd containing some namespace(such as
uof.zip
), the xml file written using the jar does not have the namespace I need and cannot be opened normally. This test process cannot be expressed by unit testing. If you need me to demonstrate it with code, I will upload a use case code package tomorrow to demonstrate why I need to use extra namespace.

@whatever098 whatever098 force-pushed the Add_Save_ExtraNamespaces branch from 55b6a1d to 16af8f6 Compare July 2, 2024 10:13
@pjfanning
Copy link
Contributor

I will not merge this unless there are checks so that the extra namespaces do not clash with the namespaces that XMLBeans generates for itself.

@pjfanning
Copy link
Contributor

Actually, I think we should abandon this and work out why XMLBeans does not generate the correct namespaces in the 1st place.

@whatever098
Copy link
Author

You are right,I will try

@whatever098
Copy link
Author

whatever098 commented Jul 6, 2024

@pjfanning I review my modified and found using setSaveImplicitNamespaces can achieve my appeal, but using this method will traverse the XML files twice, this could lead to problems in the performance of some of the scenes, so now I will use setSaveImplicitNamespaces moment, There may be time later to commit changes to the PR to add some checks during my free time.By the way, can you deal with the pr #19 as soon as possible, this matters to me.

@pjfanning pjfanning mentioned this pull request Jul 6, 2024
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.

2 participants