MessageLog::Clear()

[killerbot]

Dear all,

I'd like to suggest some minor adjustment to the core-code.

Why ?
--------
When developing my ClearCase plug-in (and an upcoming plug-in) I created a new pane in the MessageManager.
When writing stuff to such a pane, aka MessageLog, only text is added, causing the content to grow.
From time to time the user probably wants to clear this.
Sometimes with every command that could write to the log, the previous content could be automatically cleared, but even in that case I think it's a better solution in the design. I guess that adding to those logs will eventually use more and more memory as text is added.

Current Situation :
-------------------------
All those panels/Logs derive from the class MessageLog. There are 2 derived classes :
 - SimpleTextLog (which contains a wxTextCtrl)  examples : the Build Log, Code::Blocks, Debug Log
 - SimpleListLog (which contains a wxListCtrl)   examples : Search Results, Build Messages

An example when some of them get cleared is in the Compilergcc.cpp
 -> m_Log->GetTextControl()->Clear();
 -> m_pListLog()->Clear() (which talks to its wxListCtrl)

Observations :
-------------------
Althoug m_Log and m_pListLog are not exactly on the same level of inheritance, m_pListLog has I think 1 level more, it needs one visible level less to get the actual stuff cleared.

Suggestion :
-----------------
Enhance the interface of the MessageLog class with the pure function :
 - virtual void ClearLog() = 0;
Note : we could also call it just Clear instead of ClearLog


Consequences :
---------------------
1) SimpleTextLog need to implement this :
void SimpleTextLog::ClearLog()
{
   m_Text->Clear();
}

2) SimpleListLog::ClearLog()
{
  Clear();
}

--> with all these additions we did not break any code I guess.

--> We could simplify things (for example in compilergcc.cpp)
  m_Log->ClearLog();
  m_pListLog->ClearLog();

And now we can add extra functionality (where it all started with), in the context menu that shows up when you right click in the pane of the MessageLog, we could add an extra menu entry "ClearLog", which then calls the ClearLog of it's MessageLog.
The messageManager keeps track of the mappings of the PageIndex and the MessageLog through the m_LogIDs array.
If I am not misataking the context menu shown in the messagemanger is a 'enabled/disabled' version of the edit menu from cbeditor.cpp ?

As already said, maybe the name can be just Clear.


What do you think, could this be done in/before RC3 ?

I did not 'yet' created a feature request for it, since it is not just a feature request, it is also a minor design change.


kind regards,
Lieven

[mandrav]

The reason it's like it is now, is because the actual log page can be anything. It may even not contain any text control (as was the case when wxSmith used a message page for its toolbox). It wouldn't make much sense then to call Clear() on such a log page.
On the other hand, SimpleTextLog exposes a GetTextControl() function to do whatever with it...

As for the context menu, it's the default provided by the system. There are plans to add custom context menu support in MessageManager, but it's not there yet...

[killerbot]

I see, but I also noticed that in the wxWidgets pane, there's no context menu whatsoever. So here the 'Clear' is not available at all.
I think it still makes sense, because the same arguments hold for the AddLog function. In the wxWidgets pane, it makes not sense to add a Log there either.

Derived classes can always implement it empty, or to do whatever to clear what they use in their pane (same again applies for the AddLog).

Add/Remove(Clear) kinda belongs together I'd say.

[thomas]

I don't like the idea, it is perfectly alright to clear the control if the plugin developer thinks that this should be possible, that's only one member function to call. On the other hand, the amount of memory consumed by a textlog is not that large compared to other things, and a plugin developer might actually not want that the log can be cleared for some reason. ToDoListView for example, should not be able to be cleared out just like this.

Also the assumption that everything derives from either a list log or a text log is not true. I have had a custom dialog with a radio box (to turn filtering on/off), a text log, and a push button (to kill child processes), and wxSmith (as pointed out by Yiannis) is another good example.

virtual void ClearLog() = 0;
[...]
--> with all these additions we did not break any code I guess.

In fact, this will break *every* plugin that uses a log which is not derived from either SimpleTextLog or SimpleListLog.

I am not against changing an API and possibly breaking code in general (as it happens, we've just finished an API change that indeed breaks a lot of code) - but there must be a really good reason and a noticeable benefit if you do that, or all the people who have to rewrite their code will be badly p*@!ed off.
Simply saving one call to GetTextControl() does not justify a change that breaks any code in my opinion.

[killerbot]

ok ,I rest my  case (we could give it an empty implementation in the base class off course, so it does not break anything).

I do understand all your arguments, they are perfectly valid, but your arguments also should suggest then that the AddLog method should not be a pure virtual in the MessageLog base class, cause everyone should implement it now also (empty if not needed), agreed it already exists, so it will not start breaking stuff. Fuirther derivatives or aggregates could prevent the clear method by for example providing empty function or disallowing the right click menu (wxSmith : no right click menu).

I thought it was more consistent if the pair (adding/clearing) was together.

No probs, I'll implement the functionality in the plug-in itself. I have 2 questions, how do I get hold of that right click menu of the message manager, and how can I add an entry that only shows up for my panel and not in the other panels of the message manager?

cheers,
Lieven

[thomas]

I have 2 questions, how do I get hold of that right click menu of the message manager, and how can I add an entry that only shows up for my panel and not in the other panels of the message manager?

Not so easy really. None of that is done by Code::Blocks. wxTextCtrl builds this menu automatically, and it offers no API to insert items or anything (at least I know of none).
So... you will probably have to push an event handler, hijack wxEVT_RIGHT_UP and create the menu yourself. Then of course, you have to implement cut, copy, and paste yourself, too.