You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by Greg Brown <gk...@mac.com> on 2009/04/03 20:00:37 UTC

Preferences class

Hi Sandro,

I finally had a chance to take a look at your most recent Preferences code. It's still not quite in line with what I had in mind.

I just checked in an updated stub for this class. It now includes the load and save tasks I mentioned, along with some convenience methods for using the tasks (similar to pivot.wtk.media.Image). You'll see TODO comments where we need to add the JNLP persistence stuff. All of the other methods should be OK, though I have not tested them yet.

I had to make a couple of modifications to JSONSerializer to support the dot-separated key values we discussed. Previously, only get() had been supported (as getValue(), which is now deprecated), but Preferences also requires put(), remove(), and containsKey(). These methods have now been added.

Basically all we should need to do at this point is migrate your persistence code to the new stub class. Let me know if you have any questions. 

Thanks,
Greg



Re: Preferences class

Posted by Greg Brown <gk...@mac.com>.
>putting a sample value like
>    	   prefs.put("accent'inside", "accent'inside");
>doesn't work anymore (ok, it's a strange key, but is it a right json
>key ?), and gives me an
>Illegal identifier character.

This is the correct behavior. JSON paths can be represented in one of two ways:

a.b.c
["a"]["b"]["c"]

or a combination thereof

a["b"].c 

Pivot's JSON serializer requires that keys specified using the dot syntax contain only valid Java identifier characters (JavaScript enforces a similar restriction). However, you can use non-identifier characters using the bracket syntax. If you change your code to this, it should work:

prefs.put("[\"accent'inside\"]", "accent'inside");



Re: Preferences class

Posted by Sandro Martini <sa...@gmail.com>.
Hi Christopher,
first thanks for your feedback/help ...

> My understanding of JSON (based on this site: http://json.org/ ) is that the
> key can be any quoted string with the following rules:
me too.

> I seem to remember, last time I looked at the code, that Pivot's JSON parser
> allows for unquoted strings and strings quoted using other characters.  I
> think we should be sticking to the 'specification' as defined on
> http://www.json.org/
>
> This means that the existing JSON based preferences files are incorrect.
>  They currently look like this:
>
> {
>    key1 : "Value1",
>    key2 : "Value2"
> }
>
> Really they should look like this:
> {
>    "key1" : "Value1",
>    "key2" : "Value2"
> }
>
I agree with you, but so anytime a string key is not valid until it's
wrapped inside string delimiters (""), wouldn't it be better to wrap
inside "" ?
And this should be done in the JSONSerializer ...

Today I'll try to put keys delimited in my test code, maybe this works now.


Oh, one last thing:
what do you think on an utility method inside the Sewrializer that
tells if the given key is right, and also if the given object, so the
caller could think at what to do without throwing Exceptions by the
Serializer and recovering ?


Thanks,
Sandro

Re: Preferences class

Posted by Greg Brown <gk...@mac.com>.
OK - a quick Google search reveals that "common extension" is probably not the right term. Apparently many parsers try to adhere to the quoted standard strictly. However, others do not, and I would argue that it is better to support both. I wouldn't want a potential use case for Pivot to be invalidated because Pivot is unable to consume the JSON output that a non-compliant web service produces. It also seems to be a common sentiment that the JSON standard should support non-quoted keys.
 
On Wednesday, April 08, 2009, at 07:34AM, "Christopher Brind" <br...@brindy.org.uk> wrote:
>To be honest, I'm not aware of what Pivot uses being a common extension,
>I've only ever seen it in Pivot.
>What Pivot actually seems to support is actually a hybrid of CSS and JSON,
>which is OK, but it makes answering questions like Sandro's quite
>subjective, IMHO
>
>
>Cheers,
>Chris
>
>
>2009/4/8 Greg Brown <gk...@mac.com>
>
>> >This means that the existing JSON based preferences files are incorrect.
>> > They currently look like this:
>> >
>> >{
>> >    key1 : "Value1",
>> >    key2 : "Value2"
>> >}
>> >
>> >Really they should look like this:
>> >{
>> >    "key1" : "Value1",
>> >    "key2" : "Value2"
>> >}
>>
>> We support both styles. I believe this is a common extension to the JSON
>> standard. We also support single-quoted string values, because it is easier
>> to use them in WTKX:
>>
>> <Label styles="{color:'#ff0000'}"/>
>>
>>
>>
>

Re: Preferences class

Posted by Christopher Brind <br...@brindy.org.uk>.
>
> JavaScript generally drops the quotes:

var foo = {bar:100};


Sure, that's because in that example, with regards to the key, that's a
valid string and JavaScript knows you're talking about a an object property.
 With regards to the value, that's because JavaScript is doing dynamic
typing (it is a number).

This is the correct behavior. JSON paths can be represented in one of two
> ways:
>
a.b.c
> ["a"]["b"]["c"]
>
> or a combination thereof
>
> a["b"].c


I disagree - a key with period in it is valid as per the definition of a
character:

*char**any-Unicode-character-*
    *except-**"**-or-**\**-or-*
    *control-character*
*\"*
*\\*
*\/*
*\b*
*\f*
*\n*
*\r*
*\t*
*\u* *four-hex-digits*

Thus the following object is valid:

{
  "My.Key" : "My.Value"
}

The behaviour you are referring to is for use within script (specifically
JavaScript).  One of the advantages of sending JSON to a web page is that it
is very easy to convert JSON to a JavaScript object, e.g:

var json = "{ \"key\" : \"value\"}";  // could come from a remote service
var x = eval(json);

Alert(x.key);
Alert(x["key"]);
Alert(["x"].key); // this is not actually valid because ["x"] means nothing
in this context


I wouldn't want a potential use case for Pivot to be invalidated because
> Pivot is unable to consume the JSON output that a non-compliant web service
> produces.


Unfortunately, the case at the moment is that we don't seem to be able to
consume valid JSON, as per Sandro's example.   Though I agree we should be
robust enough to consume non-compliant output as well.



> It also seems to be a common sentiment that the JSON standard should
> support non-quoted keys.
>

Again, that's not something I'm aware of.  The spec is clearly laid out on
the JSON website:

http://json.org/

So, keys are always strings (which are quoted) and values could be strings
or other types (i.e. one of number, object, array, the unquoted literal
"false", the unquoted literal "true" or the unquoted literal "null").

Cheers,
Chris



2009/4/8 Greg Brown <gk...@mac.com>

> >To be honest, I'm not aware of what Pivot uses being a common extension,
> >I've only ever seen it in Pivot.
>
> JavaScript generally drops the quotes:
>
> var foo = {bar:100};
>
> I find this more readable.
>
>
>

Re: Preferences class

Posted by Greg Brown <gk...@mac.com>.
>To be honest, I'm not aware of what Pivot uses being a common extension,
>I've only ever seen it in Pivot.

JavaScript generally drops the quotes:

var foo = {bar:100};

I find this more readable.



Re: Preferences class

Posted by Christopher Brind <br...@brindy.org.uk>.
To be honest, I'm not aware of what Pivot uses being a common extension,
I've only ever seen it in Pivot.
What Pivot actually seems to support is actually a hybrid of CSS and JSON,
which is OK, but it makes answering questions like Sandro's quite
subjective, IMHO


Cheers,
Chris


2009/4/8 Greg Brown <gk...@mac.com>

> >This means that the existing JSON based preferences files are incorrect.
> > They currently look like this:
> >
> >{
> >    key1 : "Value1",
> >    key2 : "Value2"
> >}
> >
> >Really they should look like this:
> >{
> >    "key1" : "Value1",
> >    "key2" : "Value2"
> >}
>
> We support both styles. I believe this is a common extension to the JSON
> standard. We also support single-quoted string values, because it is easier
> to use them in WTKX:
>
> <Label styles="{color:'#ff0000'}"/>
>
>
>

Re: Preferences class

Posted by Greg Brown <gk...@mac.com>.
>This means that the existing JSON based preferences files are incorrect.
> They currently look like this:
>
>{
>    key1 : "Value1",
>    key2 : "Value2"
>}
>
>Really they should look like this:
>{
>    "key1" : "Value1",
>    "key2" : "Value2"
>}

We support both styles. I believe this is a common extension to the JSON standard. We also support single-quoted string values, because it is easier to use them in WTKX:

<Label styles="{color:'#ff0000'}"/>



Re: Preferences class

Posted by Christopher Brind <br...@brindy.org.uk>.
Hi Sandro,

My understanding of JSON (based on this site: http://json.org/ ) is that the
key can be any quoted string with the following rules:
*string**""*
*"* *chars* *"**chars**char*
*char chars**char**any-Unicode-character-*
    *except-**"**-or-**\**-or-*
    *control-character*
*\"*
*\\*
*\/*
*\b*
*\f*
*\n*
*\r*
*\t*
*\u* *four-hex-digits*
I seem to remember, last time I looked at the code, that Pivot's JSON parser
allows for unquoted strings and strings quoted using other characters.  I
think we should be sticking to the 'specification' as defined on
http://www.json.org/

This means that the existing JSON based preferences files are incorrect.
 They currently look like this:

{
    key1 : "Value1",
    key2 : "Value2"
}

Really they should look like this:
{
    "key1" : "Value1",
    "key2" : "Value2"
}

Cheers,
Chris



2009/4/8 Sandro Martini <sa...@gmail.com>

> Hi Greg,
> I'm converting Preferences to the new stub (using the new version of
> JSONSerializer), but now i have some little problems, starting with
> this:
>
> putting a sample value like
>           prefs.put("accent'inside", "accent'inside");
> doesn't work anymore (ok, it's a strange key, but is it a right json
> key ?), and gives me an
> Illegal identifier character.
> from the split method ... ok, but what do you think on add more detail
> to this type of errors, making it like
> Illegal identifier character ' in key "accent'inside"
> and the same thing also for values ...
>
> Ah, and also the line with
>           prefs.put("java.util.Date.toString", new
> java.util.Date().toString());
> doesn't work, and gives
> Invalid path.
> also here more infos on the problem could help ...
>
> Trying to handle these strange keys inside a "" block (like json
> strange keys if i remember well) maybe could help with this type of
> values ... if a valid json file could contains also these.
> Ok, but it's not a problem, it's only to see if could be fixed (if it
> makes sense) ...
>
>
> After commenting these lines, all worked, but now i have to move my
> methods inside the new LoadTask, SaveTask, etc ...
>
>
> Ok, but now here it's too late, i have to go in bed, let's continue later.
>
>
> Thanks,
> Sandro
>

Re: Preferences class

Posted by Sandro Martini <sa...@gmail.com>.
Hi Greg,
I'm converting Preferences to the new stub (using the new version of
JSONSerializer), but now i have some little problems, starting with
this:

putting a sample value like
    	   prefs.put("accent'inside", "accent'inside");
doesn't work anymore (ok, it's a strange key, but is it a right json
key ?), and gives me an
Illegal identifier character.
from the split method ... ok, but what do you think on add more detail
to this type of errors, making it like
Illegal identifier character ' in key "accent'inside"
and the same thing also for values ...

Ah, and also the line with
    	   prefs.put("java.util.Date.toString", new java.util.Date().toString());
doesn't work, and gives
Invalid path.
also here more infos on the problem could help ...

Trying to handle these strange keys inside a "" block (like json
strange keys if i remember well) maybe could help with this type of
values ... if a valid json file could contains also these.
Ok, but it's not a problem, it's only to see if could be fixed (if it
makes sense) ...


After commenting these lines, all worked, but now i have to move my
methods inside the new LoadTask, SaveTask, etc ...


Ok, but now here it's too late, i have to go in bed, let's continue later.


Thanks,
Sandro

Re: Preferences class

Posted by Sandro Martini <sa...@gmail.com>.
Hi Greg,
> If we do a 1.1.1 release and this class is ready in time, it could go out with that instead of 1.2.
Great, I try to find the time this week ...

> Yes, they are both checked in.
OK, I'll download now them.

> Let's take another look when you've had a chance to work with the latest Preferences stub. I believe all you will need to do at this point is plug your persistence/serialization code into the load and store tasks, so there shouldn't be too many outstanding TODOs when you are done.
Ok.

Again thanks,
Sandro

Re: Preferences class

Posted by Greg Brown <gk...@mac.com>.
>now that
>this is for the 1.2 release there is more time and finish it better in
>line with Pivot standards.

If we do a 1.1.1 release and this class is ready in time, it could go out with that instead of 1.2.

>In next days I'll try to make the other modifications, but the new
>stub class and the modified JSONSerializer are already on Subversion ?
>I'll start to see new classes and what to do today, later ...

Yes, they are both checked in.

>Oh, one last thing, have you an idea on what do to on the TODO list in
>my class ?
>Or do you think that we could keep them for the first version ?

Let's take another look when you've had a chance to work with the latest Preferences stub. I believe all you will need to do at this point is plug your persistence/serialization code into the load and store tasks, so there shouldn't be too many outstanding TODOs when you are done.





Re: Preferences class

Posted by Sandro Martini <sa...@gmail.com>.
Hi Greg,

> I finally had a chance to take a look at your most recent Preferences code. It's still not quite in line with what I had in mind.
:-( ... ok, i know that the tasks are missing as you said me, this
version was only a working version for review, to see if the rest is
ok, and in line with timings for the upcoming release, but now that
this is for the 1.2 release there is more time and finish it better in
line with Pivot standards.

In next days I'll try to make the other modifications, but the new
stub class and the modified JSONSerializer are already on Subversion ?
I'll start to see new classes and what to do today, later ...

Oh, one last thing, have you an idea on what do to on the TODO list in
my class ?
Or do you think that we could keep them for the first version ?


Thanks for your help,
Sandro