#809 Show all invalid projects in one message after loading workspace

[bluehazzard]

Right now every time codeblocks fails to load a project from a workspace it pops up an error message box to inform the user. If this workspace has 20 invalid projects the user can klick 20 message boxes away. It would be much better to inform the user after loading the workspace with one dialog about all invalid projects. A dialog like the assert dialog or error dialog of wxWidgets, with a scrollable list, would be nice..

[ollydbg]

I agree with you. I see the same issue if I open the wx3.1 32bit C::B workspace(I'm using the git, I think a lot of project need to checkout by the external svn repo)

[bluehazzard]

Ok, i made workspace loading a bit more userfriendly

some comments?

[bluehazzard]

It adds a progress dialog, that informs about the progess of loading, and if some projects are not loaded at the end a dialog that informs the user about all not loaded projects...

[Teodor Petrov]

1. Why is progressDialog on the heap? Can you put it on the stack?
2. Why do you show it even if there is 1 project?
3. Why logWindow is not deleted at the end?
4. for and if have space after the word and before the bracket.
5. Why do you not call PlaceWindow for the progress dialog?

[Teodor Petrov]

Also it will be good if you can show the name of the currently loaded project in the progress dialog.

[bluehazzard]

Why do you show it even if there is 1 project?
Because the loading of this project can also need some time. It is always better to inform the user about, why the program blocks. (the final plan would be to make something like eclipse, multiple loading bars in one log window. But i do not have the time to make it at the moment, and this will improve the user experience right now)
for and if have space after the word and before the bracket.
I forget this... Fixed. I really would like to know the reason behind this...

i have implemented all comments. Thank you!
The patch is a bit noisy... Do not know what my git has...

[Teodor Petrov]

BTW: Why do you use a logger and not just a plane string? We don't use this logging stuff much in other places. Also I'm not sure this logger works well on macOS.

[bluehazzard]

Because this shows a nice dialog, with error message icon, error message, an expand button if you want to know more, and a scrollable list with all failed projects that can be copied.
If i use a message box with a big string i can not copy the values, and if a workspace has 100 faulty projects the dialog won't fit on the screen.

This all is provided free by the wxLoggerGui and it is easy to implement (as you can see)

Also I'm not sure this logger works well on macOS.

I doubt this, but ii can not verify it. This is the main logger for wx>3.0 internal errors like in the resource system...

[Teodor Petrov]

Currently cb crashes on macOS if you run it with -v and there is something in the log.

[bluehazzard]

Is this a wxWidgets issue or a codeblocks issue?
Can you try this patch with mac os? (to have some wrong workspace, simply load the wx3.1 32bit C::B workspace of windows)

Can everybody confirm, that this implementation does not steal focus during the loading of the workspace? I am not 100% sure if the progress dialog won't steal focus on all platforms...
I have tested it on windows 10 at the moment....

[Miguel Gimenez]

The logWindow didn't show at all in my system (Windows 7), I had to add

    const bool wasLogging = wxLog::EnableLogging();

after

    wxLog* oldTarget = wxLog::SetActiveTarget(logWindow);

and

    wxLog::EnableLogging(wasLogging);

just before

    delete logWindow;

When closing the log window, if there is a window behind C::B then it becomes top window and C::B goes to the background.

Two comments: calling progressDlg.Show() is not needed, and the for still lacks the space before the bracket.

[Teodor Petrov]

1. More testing and more problems. I couldn't click anywhere on the log window when using relatively recent wx-master and gtk3.
2. wx 3.0.4 and gtk2 seems to work, but the list of errors is closed.
3. there is no logging to the main log.
4. the progress dialog can be put behind the cb window, because you're not setting correct parent. See Manager::Get()->GetAppFrame() for details.

But looking at the code I've found one major problem. The workspace loader is not supposed to have any GUI code in it. This should be abstracted away in a similar fashion to the one done in the project manager ui stuff.

I'm not sure if someone would work on console only capable C::B batch in the future, but we must not make his/her task harder!

[Miguel Gimenez]

Setting the parent of progressDlg as Teodor suggested fixes C::B jumping to background

[bluehazzard]

@Miguel Gimenez
Thank you for testing and your investigation

But looking at the code I've found one major problem. The workspace loader is not supposed to have any GUI code in it. This should be abstracted away in a similar fashion to the one done in the project manager ui stuff.

Yes. This is a problem.... Is this not a bit overkill? For the progress dialog we could simply #ifdef it out on console builds, for the log window it could be redirected to the console easily...
(The project manager is full of cbMessageBox. Is there a implementation that redirects the error message to the stdout if it is run in console mode?)

More testing and more problems. I couldn't click anywhere on the log window when using relatively recent wx-master and gtk3.

Does the wx samples also behave like this?

What is your alternative sugestion? Create a own dialog?

[Teodor Petrov]

No, it is not an overkill. Tests in the past have shown that having two builds won't work. We need a single dll/so to work for both console and ui. I guess all UI sdk related code would be moved there.

For the dialog: I have no suggestions yet. Lets tackle the bigger problem first.

Another thing we should consider is that it will be good if we can show a progress for any particular project. Loading big cmake projects isn't really fast. So moving the UI code in the src/src folder might be a mean to achieve this goal, too.

[bluehazzard]

Ok, i suspected that this ticket will go this direction....
So we basically have two problems:
1) Show the user a dialog that there are failed projects
2) Show the user a progress dialog how far the loading of the workspace is going

The requirement for both is to be able to run on a console environment, where the number two is non critical to show during console screen

For the progress dialog my suggestion is to introduce a progress manager, with a deticated log panel. There different "things" like plugins and main progress can register progresses and update them via some string identifier for example... This would make progress things pretty easy... We could introduce an global progress bar in the status bar, so sumarize all other progress bars

for 2)... no idea... If you do not like the thew log example, we have to introduce our own dialog... I do not favor the message box example....

[Teodor Petrov]

If you introduce a progress manager you'd have to consider the case where two separate peices of code would want to have separate progresses.
But if you ask me I'd rather not go the manager way. I'll just pass the progress showing object to the loader and be done with it. As simple as possible. Then as the next step do the same thing with the project loader.

After we have some uses of the progress we could think about something more complex.
https://twitter.com/mike_acton/status/1076888085256904704?lang=en

[bluehazzard]

As simple as possible.

I think the manager approach is the simplest way... We have 100 places where a progress should be showed to the user (loading workspace, loading project, loading dependencies, loading wxSmith files, cc parser, starting debugger, updating watches, compiling workspace, compiling project, probably some update manager in the future, ecc...) All this places should show the user some progress. Some are plugins, some are in the sdk and some in the src. All this progress reporting should be in one central place and showed to the user. If possible not blocking, stealing focus or being intrusive in any way. I like the eclipse approach here a lot...

For me the codebase is to large to introducing and passing an object to all this places. A singleton class like the managers are, would make thins a lot easier and less intrusive. Like the log manager. We could probably introduce this to the log manager...

This discussion goes to wide for this ticket. In this ticket i really wanted only to get rid of this 100es message boxes that pop up on loading the workspace.

For the dialog: I have no suggestions yet. Lets tackle the bigger problem first.

well i really wanted to fix only this dialog right now...

[Teodor Petrov]

1. The singleton stuff was originally the worst design decision ever made in the history of C::B.
2. All the cases you mention are different, and I guarantee you that your manager solution won't be at all simple at the end when you had handled all the rough stuff like multiple nested/interleaved progress events.
3. So please keep it simple, on place at a time...
4. I don't see a problem showing a dialog when cb is blocked waiting to load a project/solution
5. The progress from CC is different from the progress of loading. One is blocking the other is not.

[Teodor Petrov]

BTW: I'd rather prefer the current solution instead of some generic complex solution based on managers.

[bluehazzard]

OK, lets reduce the problem to more small bits, so we can improve the usability and find some common base to work with.
This patch will reduce the verbosity of the workspace loading to a minimum. This will help now and does not introduce more UI code in some SDK part.
In a second ticket we can discuss about some progress dialog and remove the whole UI code from this place. But this will take some time and i think this is a acceptable middle point for the moment.

[Teodor Petrov]

1. Does GetpMsg always return something that is non-nullptr?
2. Do not use operator+ for translatable strings. Use wxString::Format.
3. Wrap filename string in either single or double quotes.

[bluehazzard]

GetpMsg is an inline function and it is checked at the beginning of the function if it returns nullptr
It gets the Log Manager and if he is not null at the start of the function hopefully it wont get null during loading

[bluehazzard]

Fixed in revision: 11706