Hacknet-Pathfinder icon indicating copy to clipboard operation
Hacknet-Pathfinder copied to clipboard

Corrected lack of XML.ElementInfo text and element child mixing

Open Spartan322 opened this issue 3 years ago • 6 comments

ElementInfo: Added XmlNodeType Type Added IsText for validating ElementInfo as text element Added default ctor that sets Type to Element Added argument ctor to streamline construction Added TryContentAs methods for bool validation of content Added GetContentAs methods for default failed conversions of content Added TryAttributeAs methods Added GetAttributeAs methods Added AttributeAs methods for throwing exceptions Added TryAddChild for bool validation of add Added TrySetParent for bool validation of parent setting Added TrySetAttribute for bool validation attribute setting Added TryGetAttribute for bool validation and out of attribute string Added SetParent, and SetAttribute for method chaining Added GetAttribute for attribute string retrieval with default WriteToXML will write text elements with WriteString(Content) Added FromText for generating a text element from a string input Created ElementInfoStringExtensions for string handling Created ElementInfoListExtensions to take the place of ListExtensions ListExtensions is obsolete Created ElementInfoDictionaryExtensions for conversion for the attribute dictionary EventExecutor implements element and text mixing

Spartan322 avatar Apr 25 '22 01:04 Spartan322

I'd personally change the names and architecture a little -- specifically, I'd split ElementInfo into NodeInfo and ElementInfo : NodeInfo -- some operations that make sense on elements (e.g. anything related to attributes, having other NodeInfos as children) don't make sense on text nodes.

Fayti1703 avatar Apr 25 '22 14:04 Fayti1703

I'd personally change the names and architecture a little -- specifically, I'd split ElementInfo into NodeInfo and ElementInfo : NodeInfo -- some operations that make sense on elements (e.g. anything related to attributes, having other NodeInfos as children) don't make sense on text nodes.

I'm pretty sure that's a breaking change, can be considered for major version change, but I'm fairly sure that would violate semver to do otherwise and it actually could have capability to cause problems for mod devs if they used 5.1.0 and this is what anything other then 6.0.0 had in it.

Spartan322 avatar Apr 27 '22 10:04 Spartan322

I'd personally change the names and architecture a little -- specifically, I'd split ElementInfo into NodeInfo and ElementInfo : NodeInfo -- some operations that make sense on elements (e.g. anything related to attributes, having other NodeInfos as children) don't make sense on text nodes.

I'm pretty sure that's a breaking change,

Changing the class architecture (i.e. adding NodeInfo) itself shouldn't be, but you're right that changing the method signatures / collection types would be.

I guess for that we could go for something DOMian here -- Children that only contains child elements, ChildNodes that contains all nodes, including text nodes.

Fayti1703 avatar Apr 27 '22 12:04 Fayti1703

Something else: The As%Type% methods for attributes specify the name as literally attribute[key], resulting in FormatExceptions with text like Value of 'attribute[key]' is not true or false

Fayti1703 avatar Apr 27 '22 12:04 Fayti1703

Something else: The As%Type% methods for attributes specify the name as literally attribute[key], resulting in FormatExceptions with text like Value of 'attribute[key]' is not true or false

This is kind of an intended output, perhaps it should say something like Value of 'attribute[key]' is 'not a boolean value' but is not true or false instead.

Spartan322 avatar Apr 27 '22 12:04 Spartan322

You sure we don't want, say, 'attribute[foobar]' in there?

Fayti1703 avatar Apr 27 '22 12:04 Fayti1703