In the library manager selecting a library and apply and without closing selecting a library and apply does not trigger the install

[gromain]

Describe the bug
When setting up Sloeber, I tried to install several libraries from the libraries manager.
I realised that if you select several libraries, click Apply, they will install. But if you chose some more libraries and click Apply again, the newly selected ones will not install.

To Reproduce
Steps to reproduce the behavior:

1. Go to Preferences/Library Manager
2. Chose one or more library
3. Click Apply to install it
4. Chose another library and click Apply => it does nothing

Expected behavior
It should install the newly selected library.

Desktop:

• OS: Linux Manjaro stable release
• Version Sloeber v4.3.2 prepackaged

[jantje]

It only does something when you select apply and close.

[gromain]

I'm sorry, but no it doesn't. Also, when you click on Apply the first time, you can see in the progress tab the library being downloaded.

Also, It stays stuck in the preference window if I try to click on Apply and Close after clicking on Apply. The only way out from this is clicking Cancel.

[jantje]

Seems like it works better than I thought.

[jantje]

To workaround close the preferences and reopen.

[dajtxx]

I had a look and found that LibrarySelectionPage.performOk will only do something if isJobRunning is false. This is the case the first time you click Apply, but never again until the dialog is closed.

It sets isJobRunning to true and hands the work off to a background Job. But it never knows when that background job is finished and never resets isJobRunning.

One fix is to not do the library update in the background, but there is no visual indication of what's going on then. I tried it by just waiting for the job to finish using Job.join.

I think that's more in keeping with what someone would expect when pressing Apply - they should no be surprised that some processing takes place. But some sort of visual indication that the thread is busy is required. Eg

@Override
public boolean performOk() {
	Job job = new Job(Messages.ui_Adopting_arduino_libraries) {
		@Override
		protected IStatus run(IProgressMonitor monitor) {
			MultiStatus status = new MultiStatus(Activator.getId(), 0, Messages.ui_installing_arduino_libraries,
					null);
			return LibraryManager.setLibraryTree(LibrarySelectionPage.this.libs, monitor, status);
		}
	};
	job.schedule();

	BusyIndicator.showWhile(getShell().getDisplay(), new Runnable() {
		@Override
		public void run() {
			try {
				job.join();
			} catch (InterruptedException e) {
				e.printStackTrace();
			}
		}
	});

	return true;
}

[jantje]

Oops. That was pretty obvious.
Looking at my original code I'm remembered about why I don't like to do gui stuff (lack of skill)
Anyway I think there are at least two problems in the original code.

1. as you rightly noticed. isJobRunning is not reset
2. if LibraryManager.setLibraryTree returns a status not equal to ok the user should be alerted.
   I would assume a synchronous job would be a better way to fix both.
   What do you think?

[dajtxx]

I agree doing it synchronously is the way to go. I have the code I pasted above in my local repo and it works fine but as you say it needs more user feedback.

I didn't understand the code that loops through the libs and decides on whether to delete or install them. Is it looping through all libraries? It might run a lot quicker if it kept a list of pending changes based upon events from the tree and just do those. Although it's quick enough as-is that it might not be worth the effort. I'm on a Thinkpad T460, not exactly a fast laptop.

[jantje]

Is it looping through all libraries

No it is looping through all selected libraries.
In my experience most gui stuff doesn't need performance improvements efforts.

[jantje]

Any progress?

[dajtxx]

I never looked at it again, sorry. The solution requires more 'eclipse SDK' than I was planning to learn.

I'll be back in Sloeber in a couple of weeks so can take a look then. I'm working on other things until the semester starts.

[jantje]

The solution requires more 'eclipse SDK' than I was planning to learn.

I know what you mean :-)

[jantje]

This one got fixed as part of #1339