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 xmlLoadString function #809

Merged
merged 13 commits into from Sep 2, 2019
Merged

Add xmlLoadString function #809

merged 13 commits into from Sep 2, 2019

Conversation

Lpsd
Copy link
Member

@Lpsd Lpsd commented Feb 10, 2019

Currently, if you want to read XML data that is compatible with the other xml* functions, you need to create a file with the XML data (using fileCreate), then read that file using xmlLoadFile

This will allow you to read XML data via string using xmlLoadString(xmlString)

Example (client):

function init()
	print("Loading XML String [client]")

	local rootNode = xmlLoadString("<animals test='x'><monkey name='crosroad'></monkey> <turtle name='luxy'></turtle></animals>")

	if rootNode then
		local rootAttributes = xmlNodeGetAttributes(rootNode)
		print("Root Node", "Name: "..xmlNodeGetName(rootNode),  "Attributes :"..toJSON(rootAttributes))
		
		local children = xmlNodeGetChildren(rootNode)
		
		for i, childNode in ipairs(children) do
			local attributes = xmlNodeGetAttributes(childNode)
			
			print("Child #"..i, "Name: "..xmlNodeGetName(childNode), "Attributes :"..toJSON(attributes))
		end
	end
end
addEventHandler("onClientResourceStart", resourceRoot, init)

My only concerns in this commit are the following (someone who actually knows what they are doing should verify this):

Shared/XML/CXMLNodeImpl.cpp:17
Shared/XML/CXMLNodeImpl.h:62

First time looking into MTA's code, and hardly worked with C++ too, so let me know of anything dumb in my commit!

Note: I have been made aware of a branch which is ready to test, pugixml, so more than anything I am hoping this PR will speed that process up ;)

@qaisjp qaisjp added the enhancement New feature or request label Feb 11, 2019
@qaisjp
Copy link
Contributor

qaisjp commented Feb 11, 2019

What happens if someone calls xmlSaveFile on this node?

@Lpsd
Copy link
Member Author

Lpsd commented Feb 12, 2019

I'm going to look at this shortly - I'll ensure that file-related XML functions do not work with xml-nodes created via xmlLoadString

@Lpsd
Copy link
Member Author

Lpsd commented Feb 14, 2019

This should now be fixed.

function init()
	print("Loading XML String [client]")

	local rootNode = xmlLoadString("<animals test='x'><monkey name='crosroad'></monkey> <turtle name='luxy'></turtle></animals>")
	
	if rootNode then
		print("Trying to save non-file XML node")
		
		local saveXML = xmlSaveFile(rootNode)
		
		print("xmlSaveFile Result:", saveXML)
		
		print("Trying to unload non-file XML node")
		
		local unloadXML = xmlUnloadFile(rootNode)
		
		print("xmlUnloadFile Result:", unloadXML)
	end
end
addEventHandler("onClientResourceStart", resourceRoot, init)

Both will return false in the above example.

@qaisjp

This comment has been minimized.

@Lpsd

This comment has been minimized.

@Lpsd
Copy link
Member Author

Lpsd commented Feb 16, 2019

The iterator/index value of the loop within CLuaMain::DestroyXML is passed to m_XMLFiles.erase, is it worth the time simply for "readability"? (also, the loop was already like this, and I feel it falls outside the specification of my PR)

I have, however, cleaned up the loop in CLuaMain::SaveXML (one of them, at least)

Aside from this, everything seems to be working great.

@patrikjuvonen patrikjuvonen added this to the Backlog milestone Feb 20, 2019
@Lpsd
Copy link
Member Author

Lpsd commented Feb 21, 2019

Fixed some issues brought to my attention by @patrikjuvonen

XML strings are now properly parsed, to iterate through all children (instead of just the 1st)

Also added checks for broken XML strings, and error outputting

@patrikjuvonen
Copy link
Contributor

Waits for #821 to solve.

@patrikjuvonen patrikjuvonen removed their assignment May 28, 2019
@qaisjp
Copy link
Contributor

qaisjp commented May 28, 2019

@patrikjuvonen I don't think this should be assigned to 1.5.7

@patrikjuvonen patrikjuvonen modified the milestones: 1.5.7, Backlog May 28, 2019
@patrikjuvonen
Copy link
Contributor

@patrikjuvonen I don't think this should be assigned to 1.5.7

I've moved it to backlog now. We should probably check what else is left in 1.5.7 milestone and move them elsewhere if unlikely to get them completed in time.

@botder
Copy link
Member

botder commented Aug 17, 2019

The decision in pull request #821 (Pin down return value on failure) is now settled.

Lpsd and others added 5 commits August 17, 2019 17:44
Co-Authored-By: Lpsd <40902730+Lpsd@users.noreply.github.com>
Ensures that XML file functions (xmlSaveFile, xmlUnloadFile) do not work with XML nodes created without a file (via xmLoadString) and returns correctly to reflect this.
@patrikjuvonen
Copy link
Contributor

Cool, thank you!
Great job on this PR. All ready to go after 1.5.7 is released! 👍

@patrikjuvonen patrikjuvonen modified the milestones: Backlog, 1.6 Aug 17, 2019
@patrikjuvonen patrikjuvonen self-assigned this Aug 17, 2019
@patrikjuvonen patrikjuvonen merged commit b9d509a into multitheftauto:master Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants