Xml checker #1838
Xml checker #1838
Conversation
…eTypeFromData to the DataFile class. This opens the given file and checks the xml for its file type, as oposed to relying on the file extension
…on, caught a inverse logic error
…eTypeFromData to the DataFile class. This opens the given file and checks the xml for its file type, as oposed to relying on the file extension
…on, caught a inverse logic error
|
|
hmmm....... maybe pushing the same branch again wont work.
yeah, this is really confusing me. It builds and runs fine on my pc. builds and runs fine on my fork of travis. Ive just tried rebasting lets see if that helps, |
|
Ok, wip :( |
listen next time Dave. |
| } | ||
| break; | ||
| case Type::UnknownType: | ||
| if (! ( extension == "mmp" || extension == "mpt" || extension == "mmpz" || |
There was a problem hiding this comment.
A bit late in response to this but we can get back to the pretty select/case statements by creating a FileType enum down the road. I think it will have an added benefit by finally allowing us to remove the mmpz, mpt, xpf hard coded all over the place. 👍 Probably worth an enhancement request.
There was a problem hiding this comment.
At second glance Dave's logic looks OK.
It appears we register our plugins as having an extension associated... and I'm guessing the Engine::pluginFileHandling().contains() doesn't actually contain xiz for some reason. I'd guess this is actually an existing bug that surfaced with this code... but that is just a hunch.... still investigating...
There was a problem hiding this comment.
we can get back to the pretty select/case statements by creating a FileType enum down the road. I think it will have an added benefit by finally allowing us to remove the mmpz, mpt, xpf hard coded all over the place.
Im not sure I understand the requirements, This code is checking the string of the file extension. Are you suggesting something like the following
enum { mmpz, mpt, xpf };
const char *types[] = { "mmpz", "mpt", "xpf" };
.....
.....
.....
if (extension == types[mpt]) .....
It appears we register our plugins as having an extension associated... and I'm guessing the Engine::pluginFileHandling().contains() doesn't actually contain xiz for some reason. I'd guess this is actually an existing bug that surfaced with this code...
The problem with this code as it stands is xiz, are not of Type::UnknownType, the returned enum is undefined. This should not be too hard for me to get sorted :)
There was a problem hiding this comment.
Are you suggesting something like the following
Yeah, similar. It would be more like this:
switch (type) {
case Project:
case ProjectCompressed:
case ProjectTemplate:
case ZynPreset:
// etc
}|
Thanks for the careful time and consideration that went into this request. Closes #1404. |
|
:) sorry for the mess at the end |
| return true; | ||
| } | ||
| break; | ||
| case Type::InstrumentTrackSettings: |
There was a problem hiding this comment.
@softrabbit @Spekular @curlymorphic Per #1880 I think we need a better XIZ check in here somewhere... :)
There was a problem hiding this comment.
confusing with these line notes not being sequential, I believe this is covered by the above line notes.
a Resubmission of #1782