Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#15916 closed defect (fixed)

`wxTipProvider::PreprocessTip` will never be called, unless user-defined tip provider inherits `wxFileTipProvider`.

Reported by: jbbbms Owned by:
Priority: low Milestone: 3.1.0
Component: GUI-all Version: 3.0.0
Keywords: PreprocessTip, wxTipProvider Cc:
Blocked By: Blocking:
Patch: no

Description

The document here says that wxTipProvider::PreprocessTip will be called immediately after read, and before being check whether it is a comment, an empty string or a string to translate. However, in reality it will never be called, unless the user-defined tip provider inherits wxFileTipProvider, a child class of the abstract class wxTipProvider.

The document probably needs to be revised so as not to conflict with user's experience, or there shall be a wrapper method that invokes the virtual method wxTipProvider::GetTip first and then relay the new tip message returned from GetTip to wxTipProvider::PreprocessTip. If the overridable PreprocessTip returns an empty string, then the wrapper shall call GetTip again.

Change History (6)

comment:1 follow-up: Changed 11 months ago by vadz

  • Milestone set to 3.1.0
  • Priority changed from normal to low
  • Status changed from new to confirmed

This method actually seems completely useless as it can't be overridden: if you do it, you would be creating an object of your own class which doesn't use it and the only code which does use it is in wxFileTipProvider which doesn't override it.

So I think it should be just dropped to avoid the confusion. Or am I missing something?

comment:2 in reply to: ↑ 1 Changed 11 months ago by jbbbms

Replying to vadz:

So I think it should be just dropped to avoid the confusion. Or am I missing something?

Yes, me too was thinking it probably should be dropped, because if its purpose is to modify the tip messages before showing them on the screen, then why not do modification/filtering at the same time when reading from a file?

One more thing, abstract class wxTipProvider's data member #m_currentTip: size_t and its getter GetCurrentTip seem redundant in the class. Obviously not every tip provider is going to use them. For example, some may use iterator to remember the current position, and others probably just randomly select a message from the pool. Maybe m_currentTip and its getter should be moved to wxFileTipProvider that actually uses them.

comment:3 follow-up: Changed 11 months ago by vadz

We need m_currentTip to be able to implement GetCurrentTip() which we can't remove because it may be used (in fact any existing code should be using it to somehow remember the last shown tip). Of course, the fact that it's a size_t and not some opaque piece of data which could identify things other than line numbers is ugly -- clearly I wasn't in the best shape when designing this class 15 years ago :-/ But now we're stuck with it, I'm afraid.

comment:4 Changed 11 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [75744]) Deprecate wxTipProvider::PreprocessTip().

It is completely useless, and there is no reason to keep it.

Closes #15916.

comment:5 in reply to: ↑ 3 Changed 11 months ago by jbbbms

Replying to vadz:

Having grep-ed the whole source codes, the only class that uses m_currentTip is wxFileTipProvider. Unless possible 3rd party tip providers are concerned, there probably is no reason to keep m_currentTip in the abstract class wxTipProvider whose sole purpose seems only to define an interface to wxTipDialog. On the other hand, we have a (small) reason to remove m_currentTip. What if an user-defined tip provider doesn't use it, isn't it a waste of, well, 4 bytes in memory? XD

Thanks. Have a nice day.

comment:6 Changed 11 months ago by vadz

The point is that any user-defined provider would still have to use something very much like m_currentTip because it must store the initial index somewhere (ctor) and return the current index from somewhere (GetCurrentTip()). And, of course, removing it would be backwards incompatible (it's protected, not private, in a class which is meant to be inherited from), so why do it when there is really no gain (4 bytes notwithstanding)?

Note: See TracTickets for help on using tickets.