Support namespaces #176
Support namespaces #176
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #176 +/- ##
=========================================
+ Coverage 0 63.56% +63.56%
=========================================
Files 0 39 +39
Lines 0 1213 +1213
Branches 0 107 +107
=========================================
+ Hits 0 771 +771
- Misses 0 414 +414
- Partials 0 28 +28 ☔ View full report in Codecov by Sentry. |
|
Rejecting breaking change |
|
@aveXcaesar There is a simple solution. The namespaceresolver can be built when needed by obtaining the namespaces from the document node. |
|
@Meerownymous |
|
Consider this for building the manager on-the-fly foreach (var attribute in
new Filtered<XAttribute>(
att => att.IsNamespaceDeclaration,
((XElement)doc.FirstNode).Attributes()
)
)
{
if (attribute.Name == "xmlns")
{
mgr.AddNamespace("def", attribute.Value);
}
else
{
mgr.AddNamespace(attribute.Name.LocalName, attribute.Value);
}
} |
|
@Meerownymous |
|
Next time please split the PR into two. One with the feature and one with the refactoring eg. the added |
| using Xunit; | ||
| using Yaapii.Atoms.Enumerable; | ||
| using Yaapii.Atoms.Text; | ||
|
|
||
| namespace Yaapii.Xambly.Directive.Tests | ||
| { | ||
| public sealed class CopyOfDirectiveTests |
There was a problem hiding this comment.
Please add tests for the CopyOfDirective so it also works with namespaces
There was a problem hiding this comment.
I haven't seen it. You are right and I think there is something to do...
There was a problem hiding this comment.
@HerickJ-22
I would like to split the CopyOfDirective to another branch a create an new issue. Do you agree?
There was a problem hiding this comment.
Yes that is a good idea! But I think we should not merge and not publish this PR bevore the issue #192 is resolved.
…o i173-supportNamespaces
|
Wer merged hier eigentlich @HerickJ-22 |
|
@jaanpeeter I am merging in this Repo, but there is still an open discussion when this pr is going to be merged |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?** (check one with "x")
What is the current behavior? (You can also link to an open issue here)
closes #173
closes #177
What is the new behavior?
NsDirectiveXPathDirectiveDirectivesXmlResolverFromDocumentthat can resolve namespaces from given XMLDoes this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications
Other information