story.saveTiddler and extended fields

[Saq Imtiaz]

Ok, so this time around, it's extended fields that arent quite playing nice. Or more specifically, saving new tiddlers with extended fields.

The scenario: set up the newTiddler macro to use a template that creates about 6 extended fields in the new tiddler.
The problem: saving this new tiddler takes an extremely long period of time, almost long enough to make it a deal breaker in some cases.

Looking through the code drew my attention to these lines in Story.prototype.saveTiddler :

store.saveTiddler(title,newTitle,fields.text,config.options.txtUserName,minorUpdate ? undefined : newDate,fields.tags);
for (var n in fields) 
    if (!TiddlyWiki.isStandardField(n))
	store.setValue
(newTitle,n,fields[n]);

Now correct me if I am wrong, but I believe the above code results in n+1 notifications to the store for a tiddler with n extended fields! So for a tiddler with 6 fields, we are looking at 7 notifications and possibly 7 screen redraws. Surely enough, sandwiching the setValue call between store.suspendNotifications and resumeNotifications alleviates the performance hit!

So, as a result I have two questions:
1) why are we notifying the core for each field, when store.saveTiddler is already doing one notification?
2) why don't we let store.saveTiddler save the extended fields as well, instead of saving them separately? From what I've seen store.saveTiddler accepts fields as an argument.

So the above code could look like:
store.saveTiddler(title,newTitle,fields.text,config.options.txtUserName,minorUpdate ? undefined : newDate,fields.tags,fields );

I get the feeling the current behaviour is intentional and not an oversight, but what am I missing here? why do we need the multiple notifications? and if this proposed solution is not the way to go, is there anything else we can do to speed up saving tiddlers with fields?

Cheers,
Saq

--
TiddlyThemes.com : a gallery of TiddlyWiki themes.
LewcidTW (http://tw.lewcid.org ): a repository of extensions for TiddlyWiki

[Udo Borkowski]

Hi Saq,

I get the feeling the current behaviour is intentional and not an oversight, ...

I more get the feeling this is NOT intentional and an oversight (i.e. a bug ;-) ).

Your suggestion to add the store.suspendNotifications and resumeNotifications is part of the solution.

Even though the saveTiddler function has a fields parameter we cannot just pass in the fields from Story.saveTiddler. This is because the fields variable in Story.saveTiddler are all "edited" fields. I.e. it contains the standard fields like text or title and all the extended fields currently in the edit template. But it may miss other extended fields. The fields parameter for saveTiddler must contain all "extended fields" and only those. I.e. it must not contain standard fields. Therefore we need the extra loop the change the extended fields.

The suggested fix is to add a store.suspendNotifications and resumeNotifications frame around the loop and also to move the loop (with the frame) before the store.saveTiddler call. This way the extended fields are changed first and the notification handlers (called in store.saveTiddler) will also see these changes.


I raised a ticket for this: http://trac.tiddlywiki.org/tiddlywiki/ticket/278

Many thanks for reporting this,

Udo

----------
Udo Borkowski
http://www.abego-software.de

LewcidTW ( http://tw.lewcid.org ): a repository of extensions for TiddlyWiki

[Saq Imtiaz]

Hey Udo,
glad to be of help. I went back and looked at the code and you are right, store.saveTiddler can only accept standard fields in the fields argument. The proposed solution looks good!

Cheers,
Saq

LewcidTW (http://tw.lewcid.org): a repository of extensions for TiddlyWiki