Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Fix SetNamespace docs and behavior when namespace already exist #103

Closed
wants to merge 4 commits into from

Conversation

lestrrat
Copy link
Collaborator

fixes #102

Copy link

@guitarpawat guitarpawat left a comment

Choose a reason for hiding this comment

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

I pulled your library and found a possible way to fix the current namespace prefix issue by using the same namespace pointer if it already exists.
I am not familiar with CGO, so I am not sure if this is the right way to do.

Comment on lines +50 to 62
if oldNs, _ := clib.XMLSearchNs(doc, n, prefix); oldNs == 0 {
// Namespace not found, create a new one
ns, err := clib.XMLNewNs(n, uri, prefix)
if err != nil {
return err
}

if activateflag {
if err := clib.XMLSetNs(n, wrapNamespaceNode(ns)); err != nil {
return err
}
}
}
Copy link

@guitarpawat guitarpawat Jun 26, 2024

Choose a reason for hiding this comment

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

Using the same namespace pointer if it already exists

Suggested change
if oldNs, _ := clib.XMLSearchNs(doc, n, prefix); oldNs == 0 {
// Namespace not found, create a new one
ns, err := clib.XMLNewNs(n, uri, prefix)
if err != nil {
return err
}
if activateflag {
if err := clib.XMLSetNs(n, wrapNamespaceNode(ns)); err != nil {
return err
}
}
}
ns, _ := clib.XMLSearchNs(doc, n, prefix)
if ns == 0 {
// Namespace not found, create a new one
ns, err = clib.XMLNewNs(n, uri, prefix)
if err != nil {
return err
}
}
if activateflag {
if err := clib.XMLSetNs(n, wrapNamespaceNode(ns)); err != nil {
return err
}
}

count := strings.Count(dump, `xmlns:ex="https://example.com"`)
t.Logf("%s", dump)
require.Equal(t, 1, count, "expected only one namespace declaration for 'ex' prefix")
})
Copy link

@guitarpawat guitarpawat Jun 26, 2024

Choose a reason for hiding this comment

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

Add test to check namespace prefix

Suggested change
})
// root.Dump(true) should contains namespace prefix for Document
count = strings.Count(dump, "ex:Document")
require.Equal(t, 2, count, "expected Document to have 'ex' namespace prefix")
// root.Dump(true) should contains namespace prefix for Test
count = strings.Count(dump, "ex:Test")
require.Equal(t, 2, count, "expected Test to have 'ex' namespace prefix")
})

count := strings.Count(dump, `xmlns:ex="https://example.com"`)
t.Logf("%s", dump)
require.Equal(t, 2, count, "expected two namespace declaration for 'ex' prefix")
})
Copy link

@guitarpawat guitarpawat Jun 26, 2024

Choose a reason for hiding this comment

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

Add test to check namespace prefix

Suggested change
})
// root.Dump(true) should contains namespace prefix for Document
count = strings.Count(dump, "ex:Document")
require.Equal(t, 2, count, "expected Document to have 'ex' namespace prefix")
// root.Dump(true) should contains namespace prefix for Test
count = strings.Count(dump, "ex:Test")
require.Equal(t, 2, count, "expected Test to have 'ex' namespace prefix")
})

Copy link

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 11, 2024
Copy link

This PR was closed because it has been stalled for 14 days with no activity. This does not mean your PR is rejected, but rather it is done to hide it from the view of the maintainers for the time being. Feel free to reopen if you have new comments or chnages that you would like to include.

@github-actions github-actions bot closed this Jul 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dom.SetNamespace creates xmlns: in child elements
2 participants