Deserializers intolerant of unrecognised attributes in profile. #125
Description
- created by @BlueFinBima on 2019-03-27 08:06:12 +0000 UTC
Implementation of @derammo 's click suppression highlighted that both control center and profile editor are too picky about unrecognised options. Ideally Control Center should just ignore, and profile editor should show a message warning of potential loss of features if the profile is saved.
Comments:
comment by @BlueFinBima on 2019-03-29 07:24:39 +0000 UTC
Looks like this is worse than first thought as Gadroc has put a "REVISIT" tag in some of this code because it assumes a particular element order (see line 196 in Helios\Monitor.cs)
comment by @derammo on 2019-03-29 12:11:32 +0000 UTC
I put that REVISIT tag there, because this isn’t how you are supposed to read XML
Sent from my iPad
On Mar 29, 2019, at 03:24, BlueFinBima <notifications@github.commailto:notifications@github.com> wrote:
Looks like this is worse than first thought as Gadroc has put a "REVISIT" tag in some of this code because it assumes a particular element order (see line 196 in Helios\Monitor.cs)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/BlueFinBima/Helios/issues/125#issuecomment-477896368, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAx4CKgBvuCpjy85xiLwekmtION1oxfUks5vbb-3gaJpZM4cNLDS.
comment by @BlueFinBima on 2019-03-31 09:52:32 +0000 UTC
@derammo no argument about this code not being great. I just made an incorrect assumption about the origin on the comment. Happy to assign this issue to you if you want to sort it out ;-)
comment by @derammo on 2019-04-15 15:22:25 +0000 UTC
existing Helios installs out there don't have code that survives extra XML tags, so even if we fix this, it does not give forwards compatibility to existing installs
the only thing I have come up with so far is to simply not persist the value if it is not changed from the default, then a new profile will work for old installs, as long as it does not use whatever new feature is introduced (probably the best we can hope for)
see a22bde5 for example
comment by @BlueFinBima on 2019-04-16 06:54:41 +0000 UTC
@derammo This was more about making a new release of Helios's XML parsing more tolerant, I realise that not much can be done for the people out there other that being careful about not saving unchanged values. Simply tidying up the XML parsing will have a big benefit in this area.
comment by @derammo on 2020-01-21 02:23:47 +0000 UTC
I see two possible ways forward:
-
change how we read XML and port all the code in every ReadXML function to do it differently (sounds like a lot but it is possible)
-
do schema filtering, where we run the configuration through an XML schema filter before anyone gets to ReadXML so that we discard anything that isn't in the current schema version. This would mean that every time we add a new option somewhere, we would have to add it to the schema also, so it can pass
comment by @derammo on 2020-05-25 00:03:30 +0000 UTC
I have provided an alternative XML reading implementation that is compatible with ReadXML and WriteXML that I will be using for new code going forward. I doubt we will ever go back and change all the existing code, considering the risk of breaking existing profiles or older Helios versions that may depend on presence of default values and ordering of values. (see HeliosXmlModel)