296702 - Add text serializer to ChatZilla

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[James Ross]

As part of the work for doing the stored away messages, I wrote a text
serialiser thingy, which can turn a sequence of JS objects into a plain text
file, and also recontrust the JS objects from the file later.

I think we should put this in now, separately, so certain other features can be
developed on it while CVS catches up to the current releases.

Comment 1

[James Ross]

Attachment: Text Serializer going into 0.9.68.6

Comment 2

[James Ross]

Attachment: Existing code which uses the serializer

Comment 3

[:Gijs (he/him)]

Comment on attachment 185393 [details]
Text Serializer going into 0.9.68.6

>function TextSerializer(file)
>{
>    // ...
>    this.lineEnd = "\n";
>    this._initialized = true;
>}

Nit: can't we grab the line end from client? We store it there too, for use in the logging code. I know we work regardless, but it'd be nice to keep it all working for the poor notepad windows users :-)

Spelling nits (trivial):
> * Note: serialize and deserialize automatically open the file is it is not
s/is/if

>    if (!ASSERT((dir == ">") || (dir == "<"), "Bad serilization direction!"))
* serialization

> * Closes the file steam and ends reading or writing.
* stream

> * Serializes a single object into the file stream. All properties of the object
> * are stored in the stream, include properties that contain other objects.
* including


>                // Return the to previous object level.
>                obj = objs.pop();
>                if (!ASSERT(obj, "Waaa! no object level to return too!"))
* to the
* to return to!

Apart from that, it seems fine to me. :-)

Comment 4

[James Ross]

(In reply to comment #3)
> Nit: can't we grab the line end from client? We store it there too, for use in
> the logging code. I know we work regardless, but it'd be nice to keep it all
> working for the poor notepad windows users :-)

This is JS/lib code, and certainly cannot depend on FE code. The FE code which uses the object could override it, each time, of course. :)

Comment 5

[Samuel Sieb]

Comment on attachment 185393 [details]
Text Serializer going into 0.9.68.6

>TextSerializer.prototype.close =
[...]
>    return !this._open;
Isn't this redundant?  this._open will always be false.

>TextSerializer.prototype.deserialize =
>            case "start":
>                if (!params)
[...]
>                else
>                {
>                    /* Create a new object level, store the reference on the
>                     * parent, and set the new object as the current.
>                     */

need to check if we have a top-level object first, otherwise the following line will choke:

>                    obj[ecmaUnescape(params)] = n;

>                else if (params[0] == "/") // RegExp
>                {
>                    var p = params.match(/^\/(.*)\/(\w*)$/);
>                    obj[ecmaUnescape(command)] = new RegExp(ecmaUnescape(p[1]),
>                                                            p[2]);

check that the match() was succesful first?

Comment 6

[James Ross]

Attachment: New version that supports arrays

Comment 7

[James Ross]

Attachment: Diff from first version to second

Comment 8

[Samuel Sieb]

Comment on attachment 204284 [details]
New version that supports arrays

> *       TITLE My own motif
> *       URL file:///C:/Documents%20and%20Settings/User/My%20Documents/ChatZilla-motif.css
Shouldn't these lines be escaped?

> * enumeration. Thus, for loading, only items with numeric property names are
> * allowed. If an item is STARTed inside an array, and specifies no property
> * name, it will be push()ed into the array instead.
I don't see this in the code.

>function ts_deserialize()
[...]
>            // Got more data in the buffer, so split into lines.
>            // The last one doesn't count - the rest get added to the full list.
What if the last line doesn't have a line ending?

>        // 'start' and 'end' commands are special.
What about a property named "start" or "end"?

>                if (paramList.length == 0)
>                {
>                    /* We have already read one top-level object, so we return
>                     * that instead of continuing reading.
>                     */
Should probably assert if we're not at the top level.

>                    /* If we find a line that is NOT starting a new object, and
>                     * we don't have a current object, we need to store the data
>                     * on a dummy that we will omit the header for.
>                     */
>                    rv = obj = new Object();
>                    obj._noHeader = true;
Where is this used?  And it will never get returned anyways.

Comment 9

[James Ross]

Attachment: Updated some more

Comment 10

[James Ross]

Attachment: Diff from 2nd to 3rd version

Comment 11

[James Ross]

I've not addressed the missing newline at the end thing, as it is not simple and I don't think it is necessary currently.

Comment 12

[Samuel Sieb]

Comment on attachment 204883 [details]
Updated some more

>                // The property name may be enclosed in "...".
Just say "quotes", less confusing. :-)

>                // But it always escaped.
it *is* always

Comment 13

[James Ross]

Checked in (with standard tri-license) --> FIXED.

Comment 14

[James Ross]

Attachment: Fix reading quoted properties names bug

I managed to make it fail to read what it wrote - the property names are quoted, but the regexp was only for \w. This fixes it to be \S.

Comment 15

[:Gijs (he/him)]

Comment on attachment 204951 [details] [diff] [review]
Fix reading quoted properties names bug

extra r+ is me.

Comment 16

[James Ross]

Checked in --> FIXED.

Comment 17

[Nickolay_Ponomarev]

Sorry for a possibly stupid question, but why you chose to create a custom serialization format instead of using built-in toSource() method? (Maybe add whitespace if the serialized text is to be edited by users.) That approach seems to me less error-prone than a custom serializer and a regexp-based parser.

Comment 18

[James Ross]

You really think we'd be able to rely on something like that working consitently across Mozilla 1.0 to present-day? The main motivation was being able to read and edit the files easily, and trying to apply a regexp to toSource()'s output to make it readable is even less safe than writing our own system.