GNOME Bugzilla – Bug 641017
Use json instead of yaml parser
Last modified: 2011-02-23 12:24:30 UTC
Gnome applications use json as a parser/reader, so it will be more gnome compliant if orca uses json instead of yaml.
Marking at a blocker to GNOME 3 because yaml is not a blessed external dependency.
I have done this task and pushed to my gitorious personal branch [1] [1] http://gitorious.org/~jhernandez/orca-mirror/jhernandez-orca-mirror/commits/yaml-to-json
Created attachment 180063 [details] [review] patch1
Created attachment 180064 [details] [review] patch2
Created attachment 180065 [details] [review] patch3
These three patches attached fixes this bug!
(In reply to comment #6) > These three patches attached fixes this bug! Yay! Thanks Javi!! :-) I suspect that we'll need to make some additional tweaks to the test harness (we did back when we switched to yaml), so I'll look at doing that. In the meantime, I noticed that if there are existing pre-json backend settings for Orca (i.e. the yaml settings which many of our users will have now), Orca doesn't start at all. Instead it presents the traceback below. I think what might be more user-friendly is for us to catch this exception and place the user in the Orca preferences dialog automatically (i.e. like we would do if there were not any settings already established). Make sense? Traceback (most recent call last):
+ Trace 225860
_settingsManager = SettingsManager()
if not self.isFirstStart():
return self._backend.isFirstStart()
self._getSettings()
prefs = load(settingsFile)
**kw)
return _default_decoder.decode(s)
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
raise ValueError("No JSON object could be decoded")
(In reply to comment #7) > (In reply to comment #6) > > These three patches attached fixes this bug! > > Yay! Thanks Javi!! :-) Thanks to you for testing! ;) > > I suspect that we'll need to make some additional tweaks to the test harness > (we did back when we switched to yaml), so I'll look at doing that. This is not necessary since i did it, so go and check the third patch I attached! ;) In the > meantime, I noticed that if there are existing pre-json backend settings for > Orca (i.e. the yaml settings which many of our users will have now), Orca > doesn't start at all. Instead it presents the traceback below. > > I think what might be more user-friendly is for us to catch this exception and > place the user in the Orca preferences dialog automatically (i.e. like we would > do if there were not any settings already established). Make sense? Of course, let's present Orca preferences dialog, so i'll take a look. If you're working on this right now, please let me know! > > Traceback (most recent call last): > Regards!
(In reply to comment #8) > > I suspect that we'll need to make some additional tweaks to the test harness > > (we did back when we switched to yaml), so I'll look at doing that. > > This is not necessary since i did it, so go and check the third patch I > attached! ;) I read all of the patches before I tried them and commented. ;-) So there's that one little settings test, and there's about 5 hours worth of Orca regression tests. The latter depend upon Orca settings just like using Orca does. Did you run any of those tests? I mean, I would think that things like this will need to be replaced: http://git.gnome.org/browse/orca/tree/test/harness/user-settings.conf.in http://git.gnome.org/browse/orca/tree/test/keystrokes/gtk-demo/spoken_indentation.settings http://git.gnome.org/browse/orca/tree/test/keystrokes/firefox/sayAll_bug-591351-1.settings The first one uses the default settings. The second and third each have one or two settings which are different. Would you like to also update those, or shall I?
Actually, correction: The first one doesn't use the default settings either; it uses those settings required to run the test harness as expected. ;-)
(In reply to comment #9) > (In reply to comment #8) > > > > I suspect that we'll need to make some additional tweaks to the test harness > > > (we did back when we switched to yaml), so I'll look at doing that. > > > > This is not necessary since i did it, so go and check the third patch I > > attached! ;) > > I read all of the patches before I tried them and commented. ;-) I didn't want to say the opposite! :P > > So there's that one little settings test, and there's about 5 hours worth of > Orca regression tests. The latter depend upon Orca settings just like using > Orca does. Did you run any of those tests? I mean, I would think that things > like this will need to be replaced: > http://git.gnome.org/browse/orca/tree/test/harness/user-settings.conf.in > http://git.gnome.org/browse/orca/tree/test/keystrokes/gtk-demo/spoken_indentation.settings > http://git.gnome.org/browse/orca/tree/test/keystrokes/firefox/sayAll_bug-591351-1.settings > > The first one uses the default settings. The second and third each have one or > two settings which are differen Sorry Joanie, I didn't realize all these stuff > > Would you like to also update those, or shall I? I think you're the best here! ;) Regards!
(In reply to comment #11) > > Would you like to also update those, or shall I? > > > I think you're the best here! ;) Way to get yourself out of learning about our testing harness. :-P :-P Actually, I'm happy to do it. Are you going to tackle the issue regarding Orca not starting if the yaml prefs are present? If so, I'll wait for that patch, update the harness, and then run all the tests as a final sanity check.
(In reply to comment #12) > (In reply to comment #11) > > > > Would you like to also update those, or shall I? > > > > > > I think you're the best here! ;) > > Way to get yourself out of learning about our testing harness. :-P :-P > Yup, better.... xD > Actually, I'm happy to do it. Are you going to tackle the issue regarding Orca > not starting if the yaml prefs are present? Of course, five mins patch .. ;) If so, I'll wait for that patch, > update the harness, and then run all the tests as a final sanity check. Cheers! :]
Created attachment 180276 [details] [review] patch4 Here's the patch Now, if json.load raises a ValueError exception, it is catched and we present orca preferences dialog as in a firstStart
Comment on attachment 180276 [details] [review] patch4 http://git.gnome.org/browse/orca/commit/?id=a542c9b797f65fc300f44614da75caa350e26b15
Comment on attachment 180065 [details] [review] patch3 http://git.gnome.org/browse/orca/commit/?id=7396b99f801d83ea2ece43519a92096544e6c1e9
Comment on attachment 180064 [details] [review] patch2 http://git.gnome.org/browse/orca/commit/?id=0af5eee04a17a5ba6644ddb4283b05af90777610
Comment on attachment 180063 [details] [review] patch1 http://git.gnome.org/browse/orca/commit/?id=cab1b0ba704700a0949e34cda222d02504ec122e
Updated regression files as expected. http://git.gnome.org/browse/orca/commit/?id=13552b6e7401431443ef58065d25349dc748a59a
Created attachment 181688 [details] [review] This patch fixes json_backend test
Comment on attachment 181688 [details] [review] This patch fixes json_backend test http://git.gnome.org/browse/orca/commit/?id=411e2e9ff9c47d503df95c989f89a41a37dd190b