[NETBEANS-4294] Donation of original AutoSave module to Apache NetBeans

[mgraciano]

The first PR for the AutoSave module donation. Please, let me know any issue it has to be fixed asap. Thank you in advance.

[mgraciano]

Right now we do not have JavaHelp any more. To be honest I do not know how to handle that situation.

Any idea if should I just remove everything related to JavaHelp or keep that?

[mgraciano]

You also need to add this module to the build chain in nbbuild/cluster.properties also as a dependency to ide/editor.kit .

Done

[ebarboni]

Hi, we should go through ip clearance I guess. Who is owner of codebase ? Where was the previous source ?

[geertjanw]

Owner is @mgraciano.

[ebarboni]

no previous source reachable ? if no maybe consider this as contribution, not donation

[geertjanw]

It was in NetBeans contrib, all of it written by @mgraciano under OCA, so he owns it and can decide what to do with it.

[ebarboni]

in the hg.netbeans main/contrib autosave, some commit are done by others (jesse glick as an example) I've seen the question mark on the confluence transition page. I just want to be sure we are right

[geertjanw]

Yes, we are. If the code is written by someone outside Oracle or Sun (regardless of who may have tweaked it later) and it is under OCA, the writer of the code can take it and do with it what they like.

[mgraciano]

Yes, we are. If the code is written by someone outside Oracle or Sun (regardless of who may have tweaked it later) and it is under OCA, the writer of the code can take it and do with it what they like.

Yes, I wrote it in the past and Jesse did some fixes after some NB APIs chances, just to keep it working. Actually It is a honor to contribute it directly to the main distro.

[eirikbakke]

Thank you for this!

Main concerns: 1) Is it really necessary to use a module installer? If so, it should not run unless the functionality is actually enabled (is this the case?). 2) There are various listeners attached to global objects (Preferences, at least), which can lead to memory leaks. There are some (I think) incorrect attempts to use weak listeners. This would need to be reworked, ideally by removing listeners explicitly, at the right time, rather than relying on weak listeners. Weak listeners can be used, but then it needs to be very clear when the objects are really going to be garbage collected, and why they won't be garbage collected immediately. 3) This patch uses an internationalization library (omegat) that is not standard in the NetBeans codebase. It would be preferable to do internationalization in the same way as other modules. 4) This is a very big patch for relatively simple functionality; I think a lot of auto-generated files were included. Those should be removed and put in a .gitignore. (Or maybe they will go away if suggestion (3) is implemented.)

Also, would you be able to post a screenshot of the AutoSavePanel in the settings pane? Makes it easier to review the UI without building the patch ourselves.

[lkishalmi]

Well, I agree with @eirikbakke on every point, though as of the donation even the translated files have some value. So generally I'd prefer to take over the donated code, then start to shape it for the common forms. I think @matthiasblaesing wisdom would be welcome here.

[eirikbakke]

@lkishalmi If we do the donation before fixing review issues, then it should probably be done into a separate branch.

[eirikbakke]

Are these translations hand-done, or generated by Google Translate?

[lkishalmi]

@lkishalmi If we do the donation before fixing review issues, then it should probably be done into a separate branch. Yes.

[mgraciano]

All the translations were hand made. I did the Portuguese one (I leadered the open source translation effort years ago before Sun cut us off for a paid company) and Japanese was made by a Sun friend in the same time. OmegaT was what we used in the time. If you point me to the right direction I can update and donate the translation too.

[mgraciano]

The other reviews I will do this weekend. Do you have any point how to load the module correctly?

[mgraciano]

Also, would you be able to post a screenshot of the AutoSavePanel in the settings pane? Makes it easier to review the UI without building the patch ourselves.

Sure:

[eirikbakke]

I leadered the open source translation effort years ago before Sun cut us off for a paid company

Oh--was that the translation effort for all of NetBeans? That's awesome!

Manual translations we would definitively want to preserve.

Comments on the settings UI:

• In the English translation, perhaps better to use the term "autosave" instead of "save automatically". It's a well-established word for this kind of functionality (and it also takes up less space in the tab).
• I now discovered that NetBeans has a "reformat on save" option. How does this interact with the autosave function?
• To save a tab in the Options dialog, it might be good to merge the contents of the "On Save" and "Save Automatically" tabs. Just have a single tab called "Saving", and one heading called "Format on Save" and another one called "Autosave".
• How does the "save on focus" feature interact with "save files every X minutes"? If "Save files when focus is lost" is checked, are files also saved every X minutes? Perhaps get rid of the "use save automatically feature" checkbox, instead having one checkbox for "Save files when focus is lost" and one checkbox for "Save files every X minutes".

[mgraciano]

I leadered the open source translation effort years ago before Sun cut us off for a paid company

Oh--was that the translation effort for all of NetBeans? That's awesome!

Manual translations we would definitively want to preserve.

I will remove all the OmegaT files and keep the generated properties files. What do you think? I am considering to remove all the help files too since it is not used anymore. It will reduce the diff a lot.

Comments on the settings UI:

• In the English translation, perhaps better to use the term "autosave" instead of "save automatically". It's a well-established word for this kind of functionality (and it also takes up less space in the tab).

Should I do this just in the dialog or the File > Save Automatically menu item too?

• I now discovered that NetBeans has a "reformat on save" option. How does this interact with the autosave function?

As far as I can tell, it works nicely, since this module just call the already existent save actions, or the save cookie in it document for focus lost or save all action.

• To save a tab in the Options dialog, it might be good to merge the contents of the "On Save" and "Save Automatically" tabs. Just have a single tab called "Saving", and one heading called "Format on Save" and another one called "Autosave".

To merge the two tabs, probably I will need to merge both modules. Is it desirable? If so, I can do this without a problem.

• How does the "save on focus" feature interact with "save files every X minutes"? If "Save files when focus is lost" is checked, are files also saved every X minutes? Perhaps get rid of the "use save automatically feature" checkbox, instead having one checkbox for "Save files when focus is lost" and one checkbox for "Save files every X minutes".

Both actions are designed to be independent. If both are checked the save operation will be invoked for the event that happens first. Do you mean the UI to be something like this?

[mgraciano]

• In the English translation, perhaps better to use the term "autosave" instead of "save automatically". It's a well-established word for this kind of functionality (and it also takes up less space in the tab).

Is it what you mean?

[mgraciano]

I was thinking two options for what you asked:

1. Move the options back do Miscellaneous and treat this feature out of Editor umbrella (This could be a feature for platform applications too, since 'save' operations are not specifically tied to source editor;
2. Change the patch drastically and move the option to org.netbeans.modules.options.editor. Here included the file where the properties are persisted as well;

What would be the desirable thing to do?

[lkishalmi]

Well, I'd keep this one as a separate module as not all NetBeans based applications necessarily shall support autosave, so they can opt this out by not activating this module. Also keep the tab inside the Editor Options, because that's where it is belong.

[mgraciano]

Well, I'd keep this one as a separate module as not all NetBeans based applications necessarily shall support autosave, so they can opt this out by not activating this module. Also keep the tab inside the Editor Options, because that's where it is belong.

Ok. I am working on the other review points and will have a new version by tomorrow.

[mgraciano]

Ok, I agreed and integrated most of the suggested changes (most of listeners were unnecessary and were removed.

There is only one point I would like to discuss yet, it is the menu and toolbar option. Since I refactored the Options UI, the save on lost focus option is not anymore a chilld option for autosave at all, so only one option at menu/toolbar makes no sanse to me anymore, where I have 2 options: . Remove the menu/toolbar action at all; . Change the Options to what it was originally.

Both changes are quick and I will be glad to do as we decide.

[mgraciano]

I only had a quick look at this, but this is my reply: The code looks ok to merge, as long as the statements are true. The license header says, that the code is licensed to the ASF under contributor agreement, that means that @mgraciano needs to be in a position to do this, i.e. @mgraciano needs to be the sole author or contributions were so minimal, that they don't form a copyright on their own. If that is the case all is good, if not, the offending parts need to be removed.

I saw at least one binary (auto_save.png) for which license information is missing. Please add a licenseinfo.xml file to add that info (for an example see ide/db.dataview/licenseinfo.xml and https://cwiki.apache.org/confluence/display/NETBEANS/Legal+FAQ#LegalFAQ-licenseinfoPerfileinfo-licenseinfo.xml)

For the code itself: While unifying the translations is important, I would not see it as a breaking problem, as it can be fixed after integration.

After the integration I can continue to do the fixes as necessary, even the translation. I intend to create some testes too in the future.

[eirikbakke]

I will remove all the OmegaT files and keep the generated properties files. What do you think? I am considering to remove all the help files too since it is not used anymore. It will reduce the diff a lot.

I think that would make sense, in my opinion.

Should I do this just in the dialog or the File > Save Automatically menu item too?

I hadn't noticed that menu item. I don't think it should be there--if every option got its own menu item, the menus would get very long. Just having the settings in the options dialog is good enough. (But regardless, yes, I would always use the word "Autosave" instead of "Save Automatically".)

(So, to your later question, yes, I would suggest removing the menu and toolbar action. But you could still keep the code for it, if you want--just remove the action registration. That way, the user can still drop it in a customized toolbar if they want, or we can use it later if we change our mind.)

To merge the two tabs, probably I will need to merge both modules. Is it desirable?

I see--perhaps not. I'd go with lkishalmi's opinion here.

[mgraciano]

I am sorry for the long delay. Hope everyone is ok in crazy days.

Should I do this just in the dialog or the File > Save Automatically menu item too?

I hadn't noticed that menu item. I don't think it should be there--if every option got its own menu item, the menus would get very long. Just having the settings in the options dialog is good enough. (But regardless, yes, I would always use the word "Autosave" instead of "Save Automatically".)

(So, to your later question, yes, I would suggest removing the menu and toolbar action. But you could still keep the code for it, if you want--just remove the action registration. That way, the user can still drop it in a customized toolbar if they want, or we can use it later if we change our mind.)

Great idea! I have just pushed this changed.

To merge the two tabs, probably I will need to merge both modules. Is it desirable?

I see--perhaps not. I'd go with lkishalmi's opinion here.

Agreed too. It can be reviews int he future, not a big deal.

I have a little question. After we push it to the default distro, I wish to build it in a standalone way and include in the 11.X update center. Where can a I find any documentation how to proceed? More specifically to the Plugin Portal.

[lkishalmi]

Well the NBM will be built in 12.1 might be just added to the 12.0 update center, or an other UC. But that's sure you will be able to download it separately.

[lkishalmi]

Approved the changes, however I think modifying the editor.kit dependencies is not required since the module is eager loaded. It will be automatically loaded if present. I've just tested it. @JaroslavTulach could you confirm that?

[mgraciano]

I have published a fork of this plugin to Plugin Portal and a user asked to change the time precision from minutes do milliseconds or seconds. VS Code has a ms precision. I personally think it is too much, but seconds could be feasible. I will probably test it there, but since we have not integrated it yet to main base, what do you think of this?

I did some tests and found no objections to this request in seconds precision, but I haven't tested it with features like 'Compile on Save' and 'Deploy on Save'.

[lkishalmi]

Well, millisecond precision is a no-no. Even seconds shall have a lower limit of 10 maybe 5 seconds as it could cause bad things on Compile on Save / Deploy on Save.

[humphreygao]

If you want to save every 5 minute, just multiply it by 60.

I find the font weight affects the width of editor tab, when the file is dirty, the tab width becomes longer. And once the file is saved, the tab will restore its original length. This change may distract attention. Was it possible to keep the editor tab width unchanged?

[mgraciano]

Well, millisecond precision is a no-no. Even seconds shall have a lower limit of 10 maybe 5 seconds as it could cause bad things on Compile on Save / Deploy on Save.

My question is, should I change it now or we can proceed with this donation as it is? I am afraid exhaustive testing should be done on it what could keep this even longer.

[mgraciano]

If you want to save every 5 minute, just multiply it by 60.

I find the font weight affects the width of editor tab, when the file is dirty, the tab width becomes longer. And once the file is saved, the tab will restore its original length. This change may distract attention. Was it possible to keep the editor tab width unchanged?

@humphreygao could you file a issue requestion it

If you want to save every 5 minute, just multiply it by 60.

I find the font weight affects the width of editor tab, when the file is dirty, the tab width becomes longer. And once the file is saved, the tab will restore its original length. This change may distract attention. Was it possible to keep the editor tab width unchanged?

Hello. AFAIK there is not such option. I think it is unrelated to this donation and option. I just create a issue requesting this https://issues.apache.org/jira/browse/NETBEANS-4488. Please, add your thoughts there.

[lkishalmi]

Finally merged as this one is probably the most discussed PR in NetBeans history...