This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
See T10 & Y06 in issue #36122 for background. If the 'PersistenceType' client property on TopComponent is left as is, it should at least be listed in the arch description as stability level 'devel' or stronger (not 'private'), and also mentioned in the TopComponent class Javadoc. Consider however making it a method in TopComponent, e.g. pu st fi int PERSISTENCE_NEVER = 0; pu st fi int PERSISTENCE_ONLY_OPENED = 1; pu st fi int PERSISTENCE_ALWAYS = 2; /** * Declare when this TC should be stored to disk. * Return value should be constant * over a given TC's lifetime. * @return one of P_X constants */ public int getPersistenceType() { // first check for 'PersistenceType' client // prop for compat // else default to P_ALWAYS? } Ideally (IMHO) the default should be P_O_O (to make it easiest to write code that does not leave junk settings files in the user dir) but that may cause compatibility problems for old modules expecting even closed components to be persisted. TBD whether that is really a serious enough incompatibility to prevent the default from being changed. Should also document that for P_N, you need not override read/writeExternal.
Probably suitable for fast-track review.
How is it with this?... is it really necessary now? Marek, it's your area, passing to you so I don't forget about this.
What do other think about it? Shall we start FASTREC for this? Regarding default value I think it should stay unchanged ie. P_A. settings files are deleted anyway when they are not used ie. referenced from wstcref or wstcgrp. (So TC must be member of some group or be docked opened or closed to some mode to survive in winsys.)
I think this should be done, fast-track review seems to be quite appropriate. You may assign the issue to apireviews@netbeans.org and add API_REVIEW_FAST keyword.
Created attachment 12584 [details] Diff for this issue
Diff is attached. To keep backward compatibility with client property I chose PERSISTENCE_ALWAYS as default persistence type.
Going to commit given diff tomorrow if there are no objections.
Need also to patch openide/arch/arch-openide-windowsystem.xml#exec-component, openide/api/doc/changes/apichanges.xml, openide/openide-spec-vers.properties, perhaps openide/api/doc/org/openide/doc-files/upgrade.html. Don't forget @since in patch. Also consider printing a brief runtime warning if the method is not overridden to encourage people to specify the behavior using the method; e.g. private static final Set/*<Class>*/ warnedClasses = new WeakSet(); public int getPersistenceType() { if (warnedClasses.add(getClass())) { ErrorManager.getDefault().log(ErrorManager.WARNING, "Note - " + getClass().getName() + " ought to override getPersistenceType() rather than using the client property or accepting the default."); } // as before... }
I would print warning only if client property PersistenceType is not null. I think using default behaviour of TopComponent (not overriding getPersistenceType) is ok.
Re. the previous comment: Yes, that's OK; I only suggested keeping the warning even for those TC subclasses which do nothing special, because the default value (ALWAYS) is a somewhat dangerous default (even if necessary for backwards compatibility). You need to do plenty of special work to make persistence really work correctly, which many TC's do not need at all (NEVER is fine); and WHEN_OPEN is usually a better choice than ALWAYS since ALWAYS contributes to pollution of the user directory and is rarely of any value to the user. Probably many existing TC's (esp. in third-party modules) are using ALWAYS simply because the authors were not aware of any other possibilities. Keeping the warning on unless the method is overridden simply ensures that people who really wanted ALWAYS will show that explicitly in their code.
Ok. I did it as Jesse suggested. Only I do not print warning when it is instance of TopComponent not subclass. I also updated documentation.
Looks great, thanks.