Opened 3 years ago

Closed 3 years ago

Last modified 18 months ago

#13296 closed enhancement (wontfix)

wxDocument::GetCurrentView()

Reported by: troelsk Owned by:
Priority: low Milestone:
Component: GUI-all Version: stable-latest
Keywords: wxDocument wxView wxDocManager Cc:
Blocked By: Blocking:
Patch: yes

Description

A convenience function to get the current, or active, view for the document.
If no view is activated, the first view is returned instead.

Attachments (1)

getcurrentview.patch download (1.7 KB) - added by troelsk 3 years ago.
Trunk

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by troelsk

Trunk

comment:1 Changed 3 years ago by vadz

  • Priority changed from normal to low
  • Status changed from new to infoneeded_new

Could you please explain what's the difference between the current, active and first views? Not that we support multiple views properly anyhow but how is exactly "current" is defined? The one having focus would be a natural definition but isn't this already what "active" means?

TIA!

comment:2 follow-up: Changed 3 years ago by troelsk

  • Status changed from infoneeded_new to new

You are referring to GetActiveView() that has popped up in trunk? I wasn't aware of this method until now. It is not clear when it was introduced from the changelog, or why exactly.
http://svn.wxwidgets.org/viewvc/wx/wxWidgets/trunk/include/wx/docview.h?view=log
Now I'm asked to explain about the reasoning behind why wxDocmanager::GetCurrentView() and this new GetActiveView() method was introduced. And priority of this ticket has become "low". This is obviously a go away message and nothing else.

comment:3 in reply to: ↑ 2 Changed 3 years ago by vadz

Replying to troelsk:

You are referring to GetActiveView() that has popped up in trunk? I wasn't aware of this method until now.

And I didn't remember it was new even though I did it myself in r58496, sorry. I didn't understand this code back then neither though, as my comment says I just restored the old code (albeit in a method with a different name) because it was needed to fix #9518.

Now I'm asked to explain about the reasoning behind why wxDocmanager::GetCurrentView() and this new GetActiveView() method was introduced. And priority of this ticket has become "low". This is obviously a go away message and nothing else.

No, not at all. I did forget that it was myself who added the latter and should have looked in the history but otherwise I still think the difference between GetCurrentView() and GetActiveView() is confusing. In wxDocManager case the latter is supposed to go to more trouble than the former but this doesn't really help me to understand which name is more appropriate to the new method. I also still don't know if it guarantees that the view returned by it has focus or not, which would be useful to know.

It's not a "go away" message in any way or form but I do prefer to at least avoid adding more methods with ill-defined semantics to docview classes, even if I can't do much/anything about those that are already there.

Finally, the priority of this is indeed low because I don't see it as fixing any serious problem or adding very useful functionality. But this doesn't mean I am not going to try to take care of it if I realize that it's useful for wx.

Personally I'd really prefer to do something about the existing methods, i.e. at least document their behaviour better, before adding this one.

comment:4 Changed 3 years ago by troelsk

Personally I'd really prefer to do something about the existing methods,
i.e. at least document their behaviour better, before adding this one.

Makes sense but is somewhat off topic. About this ticket,
wxDocManager::GetCurrentView() is in there, since the beginning of times, so adding the "same" method to wxDocument is consistent API, which seems a good priority. Even wx users will grow tired of typing doc->GetCurrentManager()->GetCurrentView(); enabling doc->GetCurrentView() should hurt noone.

comment:5 Changed 3 years ago by vadz

It just bothers me that I have no idea what does the existing GetCurrentView() return. Of course, I realize that sometimes it will return the view that has focus. And it looks like this isn't always the case. But when exactly does it fail? Can it return NULL and if so when exactly?

All these questions apply to the new function as well and there is also an additional one: should it use GetCurrentView() or GetActiveView() for its implementation?

comment:6 Changed 3 years ago by troelsk

It just bothers me that I have no idea what does the existing GetCurrentView() return

Well, it simply returns the wxView instance that the user specified earlier on when invoking wxDocManager::ActivateView(); this the user will make happen normally via calling wxView::Activate() in a EVT_SET_FOCUS() or EVT_CHILD_FOCUS() event handler.
This is simple enough, and simple is good.

comment:7 Changed 3 years ago by vadz

So will this just return the last view we called Activate() for? If so, it really would make more sense to call it GetActiveView(), wouldn't it?

But then why should it fall back to GetFirstView()? Shouldn't it rather return NULL then? But, of course, then it should be called GetCurrentView(). And the version with fallback should be GetActiveView() for consistency with the wxDocManager methods.

I really don't know how can you say it's simple. It seems anything but to me.

comment:8 Changed 3 years ago by vadz

  • Resolution set to wontfix
  • Status changed from new to closed

I don't want to apply a patch which confuses me so much. Chances are it will confuse other people too. When in doubt, let's at least do no harm.

comment:9 Changed 18 months ago by VZ

(In [73925]) Add public wxDocManager::GetAnyUsableView().

This method tries to find the current view harder than GetCurrentView() and
always returns a non-NULL view if there are any open documents at all.

This is used by wxDocManager internally to find the view to apply the user
commands to and will also be needed in the upcoming changes outside of
wxDocManager, so just make this method public, as it seems that it could be
useful in user code too, especially if we could use some better fallback than
the first opened document (e.g. the last document the user interacted with
would be better).

This also clarifies the confusion between GetCurrentView() and GetActiveView(),
see #13296.

Note: See TracTickets for help on using tickets.