#24704 ensure xml parse can cope with comments after empty notes #24709
Conversation
| @@ -157,7 +157,7 @@ static std::pair<XMLNode*, XmlStreamReader::TokenType> resolveNode(XMLNode* curr | |||
| } | |||
|
|
|||
| XMLNode* sibling = currentNode->NextSibling(); | |||
| if (!sibling || sibling->ToElement() || sibling->ToText()) { | |||
| if (!sibling || sibling->ToElement() || sibling->ToText() || sibling->ToComment()) { | |||
| return m_xml->doc.ErrorLineNum(); | ||
| int64_t lineNum = m_xml->doc.ErrorLineNum(); | ||
| if (lineNum == 0 && m_xml->node) { | ||
| lineNum = m_xml->node->GetLineNum(); |
There was a problem hiding this comment.
try to return a more helpful line number based on the most recently processed node
| @@ -1088,6 +1088,9 @@ Err MusicXMLParserPass1::parse() | |||
|
|
|||
| if (!found) { | |||
| m_logger->logError(u"this is not a MusicXML score-partwise file, node <score-partwise> not found", &m_e); | |||
| if (!m_e.errorString().isEmpty()) { | |||
| m_errors += errorStringWithLocation(m_e.lineNumber(), m_e.columnNumber(), m_e.errorString()) + '\n'; | |||
| } | |||
There was a problem hiding this comment.
ensure at least some useful error info shown if score-partwise can't be detected due to an unrecoverable low-level parsing error
| @@ -2923,7 +2923,7 @@ void MusicXMLParserPass2::staffDetails(const String& partId, Measure* measure) | |||
|
|
|||
| staff_idx_t staffIdx = m_score->staffIdx(part) + n; | |||
|
|
|||
| StringData* t = new StringData; | |||
| StringData stringData; | |||
There was a problem hiding this comment.
memory leak
Admittedly the term "stringData" isn't ideal, as it's not referring to "string" as in a C++ type, rather a guitar string!
| } | ||
| } else if (stringData.strings() > 0) { | ||
| m_logger->logError(u"trying to change string data for non-TAB staff (not supported)", &m_e); |
There was a problem hiding this comment.
incorrect logic, this was being logged as an error for any staff that wasn't a TAB staff (didn't affect what was shown to user though)
Backport of musescore#24709, the only relevant part for Mu3, a memory leak and an incorrect logic for logging an error, both in importmxmlpass2.cpp
|
Thanks for this fix - the changes in xmlstreamreader and pass1 look good to me. I'd also like to get this in the next patch - would you be able to separate this into a separate commit from the changes in pass2? I also think the error is totally unneeded in pass2 and can just be removed. |
Happy to remove the error around stringData but is there a reason you don't want the pass2 changes in the same commit? There's no good reason for that StringData to be allocated on the heap (then never freed). |
|
It'd be too much for 4.4.3 I guess? |
|
I'm not even fussed if it goes in 4.4.3, I just wrote my own tool to clean up Musicxml files first (I had to anyway as the timing was incorrect for notes in staff 2). |
|
Rebase needed. Seems this at least partly got merged into 4.4.3. |
Backport of musescore#24709, the only relevant part for Mu3, a memory leak and an incorrect logic for logging an error, both in importmxmlpass2.cpp
|
Still needs a rebase |
Backport of musescore#24709, the only relevant part for Mu3, a memory leak and an incorrect logic for logging an error, both in importmxmlpass2.cpp
cbjeukendrup
force-pushed
the
fix_music_xml_parsing
branch
from
6f23073 to
3920c57
Compare
|
Rebased |
Backport of musescore#24709, the only relevant part for Mu3, a memory leak and an incorrect logic for logging an error, both in importmxmlpass2.cpp
Resolves: #24704