{"action":"created","issue":{"url":"https://api.github.com/repos/musescore/MuseScore/issues/2386","repository_url":"https://api.github.com/repos/musescore/MuseScore","labels_url":"https://api.github.com/repos/musescore/MuseScore/issues/2386/labels{/name}","comments_url":"https://api.github.com/repos/musescore/MuseScore/issues/2386/comments","events_url":"https://api.github.com/repos/musescore/MuseScore/issues/2386/events","html_url":"https://github.com/musescore/MuseScore/pull/2386","id":133163144,"number":2386,"title":"fix #97931: system initial barline type lost on reload","user":{"login":"MarcSabatella","id":1799936,"avatar_url":"https://avatars.githubusercontent.com/u/1799936?v=3","gravatar_id":"","url":"https://api.github.com/users/MarcSabatella","html_url":"https://github.com/MarcSabatella","followers_url":"https://api.github.com/users/MarcSabatella/followers","following_url":"https://api.github.com/users/MarcSabatella/following{/other_user}","gists_url":"https://api.github.com/users/MarcSabatella/gists{/gist_id}","starred_url":"https://api.github.com/users/MarcSabatella/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MarcSabatella/subscriptions","organizations_url":"https://api.github.com/users/MarcSabatella/orgs","repos_url":"https://api.github.com/users/MarcSabatella/repos","events_url":"https://api.github.com/users/MarcSabatella/events{/privacy}","received_events_url":"https://api.github.com/users/MarcSabatella/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":6,"created_at":"2016-02-12T05:51:44Z","updated_at":"2016-02-13T17:57:23Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/musescore/MuseScore/pulls/2386","html_url":"https://github.com/musescore/MuseScore/pull/2386","diff_url":"https://github.com/musescore/MuseScore/pull/2386.diff","patch_url":"https://github.com/musescore/MuseScore/pull/2386.patch"},"body":"We were writing the value as an int but reading itexpecting a string.  Since regular barlines read and write the type as a string, I changed the code here to be consistent with that."},"comment":{"url":"https://api.github.com/repos/musescore/MuseScore/issues/comments/183714612","html_url":"https://github.com/musescore/MuseScore/pull/2386#issuecomment-183714612","issue_url":"https://api.github.com/repos/musescore/MuseScore/issues/2386","id":183714612,"user":{"login":"MarcSabatella","id":1799936,"avatar_url":"https://avatars.githubusercontent.com/u/1799936?v=3","gravatar_id":"","url":"https://api.github.com/users/MarcSabatella","html_url":"https://github.com/MarcSabatella","followers_url":"https://api.github.com/users/MarcSabatella/followers","following_url":"https://api.github.com/users/MarcSabatella/following{/other_user}","gists_url":"https://api.github.com/users/MarcSabatella/gists{/gist_id}","starred_url":"https://api.github.com/users/MarcSabatella/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MarcSabatella/subscriptions","organizations_url":"https://api.github.com/users/MarcSabatella/orgs","repos_url":"https://api.github.com/users/MarcSabatella/repos","events_url":"https://api.github.com/users/MarcSabatella/events{/privacy}","received_events_url":"https://api.github.com/users/MarcSabatella/received_events","type":"User","site_admin":false},"created_at":"2016-02-13T17:57:23Z","updated_at":"2016-02-13T17:57:23Z","body":"1) I think I have added the P_TYPE::BARLINE_TYPE correctly, but I have to say I am more concerned I got something wrong than I was about my original fix.\r\n\r\n2) I guess the advantage of using properties in this way is that centralize the places that need to know about the specifics of the tags and their values.  But looking at the code, I'm not so impressed with how this works in practice.  In getProperty(), most of the properties that are represented internally as enums but written as strings are handled via a bunch of explicit tests for specific literals, and this still needs to be synchronized with the corresponding code in Xml::tag().  For the new BARLINE_TYPE, I decided to leverage the existing code to use the table of names rather than explicit tests against string literals in the body of the code.  So in theory, it's a bit safer than the other types, but it still seems kind of messy to me.  I guess that's just how it is, though.\r\n\r\n3) Regular barlines use P_ID::SUBTYPE for this,  which is also shared by other elements as an \"int\", so I can't immediately convert that to using the new P_TYPE::BARLINE_TYPE.  Doing it right would, I think, would mean making a new P_ID::BARLINE_TYPE property (of type P_TYPE::BARLINE_TYPE) and converting both regular barlines and the system initial barline to use this property instead of P_ID::SUBTYPE and P_ID::SYSTEM_INITIAL_BARLINE_TYPE respectively.  I guess it could still use the \"subtype\" tag, which would represent an incompatible change as far as the system initial barline is concerned, but since that doens't work currently anyhow, it would be no real loss.  Or we could start using a new tag for this since \"subtype\" is normally read/written as a string rather than an int, but then we need break compatibility.  No doubt this is all possible, but it would require a larger change to the code than I think is warranted right now just to fix this one issue with system initial barlines.  Maybe that should wait until we do a version number bump.\r\n\r\n4) Because this new property type is only used here, I didn't think it made sense to handle it in Element::readProperties(), but there is no Measure::readProperties(), so I'm still reading the tag literally in Measure::read(), but at least I can use getProperty().\r\n\r\n5) I'm not squashing this yet in case after review it is decided to just keep the original simpler change, but of course I can do so once I am sure I've got it right."}}