fix for cshelp.cpp (PR #25809)
StevenPHirshman
Modified cshelp.cpp file so that context help click on toolbar button returns button ID, NOT toolbar ID.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25809
Commit Summary
- 5d89d67 Update cshelp.cpp
- dc9078f Merge branch 'master' of https://github.com/StevenPHirshman/wxWidgets
File Changes
(1 file)
- M src/common/cshelp.cpp (17)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25809.patch
- https://github.com/wxWidgets/wxWidgets/pull/25809.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25809)Thanks, but I'd really prefer to have a virtual function instead of using dynamic cast like this.
And, of course, as you can see from the CI failures, the current version doesn't compile due to missing wx/toolbar.h include (but it won't be necessary if/when you change the code to use the virtual function).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)***@***.***
From: VZ ***@***.***>
Sent: Thursday, September 18, 2025 7:36 AM
To: wxWidgets/wxWidgets ***@***.***>
Cc: StevenPHirshman ***@***.***>; Author ***@***.***>
Subject: Re: [wxWidgets/wxWidgets] fix for cshelp.cpp (PR #25809)
[Image removed by sender.]vadz left a comment (wxWidgets/wxWidgets#25809)<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3306976225>
Thanks, but I'd really prefer to have a virtual function instead of using dynamic cast like this.
And, of course, as you can see from the CI failures, the current version doesn't compile due to missing wx/toolbar.h include (but it won't be necessary if/when you change the code to use the virtual function).
—
You are receiving this because you authored the thread.Message ID: ***@***.***>
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25809)Strange – I was able to build the library with no errors.
You are probably using PCH. Please open this PR in your browser, you will see all the CI errors.
Any suggestion WHERE the virtual function could go?
In wxWindow, of course, as I wrote in this comment.
P.S. If you reply by email, could you please trim the quoted text because it looks really poorly on the web page (again, please open it in your browser to see it).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)Added a virtual function to window.h called SetHelpEventPos. It does nothing except for the wxtoolbar base case, where it parses the button clicked. No static or other casts are needed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- 45aefbf added virtual SetHelpEventPos
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)https://github.com/StevenPHirshman/wxWidgets/commit/45aefbf1eaa8232abaed1338f1e78d3bba7e072e
***@***.***
From: VZ ***@***.***>
Sent: Thursday, September 18, 2025 8:24 AM
To: wxWidgets/wxWidgets ***@***.***>
Cc: StevenPHirshman ***@***.***>; Author ***@***.***>
Subject: Re: [wxWidgets/wxWidgets] fix for cshelp.cpp (PR #25809)
Strange – I was able to build the library with no errors.
You are probably using PCH. Please open this PR in your browser, you will see all the CI errors.
Any suggestion WHERE the virtual function could go?
P.S. If you reply by email, could you please trim the quoted text because it looks really poorly on the web page (again, please open it in your browser to see it).
—
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz requested changes on this pull request.
Thanks, I don't really understand what is the advantage of doing it like this rather than using GetHelpIdAtPoint() as I thought to do it, but why not.
The real problem for me is that:
- This function name is misleading, it sets the ID and (not just the) position.
- IMO it shouldn't set the position at all, it doesn't make much sense to have it in client coords for the toolbars and screen coords for everything else.
So could you please rename the function to something like AdjustHelpEvent() and remove SetPosition() call from it unless there is some good reason to do it that I'm missing?
> @@ -1448,6 +1448,9 @@ class WXDLLIMPEXP_CORE wxWindowBase : public wxEvtHandler
return GetHelpTextAtPoint(wxDefaultPosition, wxHelpEvent::Origin_Unknown);
}
+ //sph091825
Please don't do this, we use version control for this. Just imagine what this file would look like if each of hundreds of contributors to it left such lines in it.
⬇️ Suggested change- //sph091825 + //sph091825
> @@ -1448,6 +1448,9 @@ class WXDLLIMPEXP_CORE wxWindowBase : public wxEvtHandler
return GetHelpTextAtPoint(wxDefaultPosition, wxHelpEvent::Origin_Unknown);
}
+ //sph091825
+ virtual void SetHelpEventPos(wxHelpEvent& WXUNUSED(event)) { return; }
This is trivial, but why have this useless return?
- virtual void SetHelpEventPos(wxHelpEvent& WXUNUSED(event)) { return; }
+ virtual void SetHelpEventPos(wxHelpEvent& WXUNUSED(event)) { }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)***@***.***
From: VZ ***@***.***>
Sent: Thursday, September 18, 2025 11:46 AM
To: wxWidgets/wxWidgets ***@***.***>
Cc: StevenPHirshman ***@***.***>; Author ***@***.***>
Subject: Re: [wxWidgets/wxWidgets] fix for cshelp.cpp (PR #25809)
Thanks, I don't really understand what is the advantage of doing it like this rather than using GetHelpIdAtPoint() as I thought to do it, but why not.
The real problem for me is that:
2. IMO it shouldn't set the position at all, it doesn't make much sense to have it in client coords for the toolbars and screen coords for everything else.
So could you please rename the function to something like AdjustHelpEvent() and remove SetPosition() call from it unless there is some good reason to do it that I'm missing?
________________________________
> @@ -1448,6 +1448,9 @@ class WXDLLIMPEXP_CORE wxWindowBase : public wxEvtHandler
return GetHelpTextAtPoint(wxDefaultPosition, wxHelpEvent::Origin_Unknown);
}
+ //sph091825
Please don't do this, we use version control for this. Just imagine what this file would look like if each of hundreds of contributors to it left such lines in it.
⬇️ Suggested change
- //sph091825
+ //sph091825
________________________________
> @@ -1448,6 +1448,9 @@ class WXDLLIMPEXP_CORE wxWindowBase : public wxEvtHandler
return GetHelpTextAtPoint(wxDefaultPosition, wxHelpEvent::Origin_Unknown);
}
+ //sph091825
+ virtual void SetHelpEventPos(wxHelpEvent& WXUNUSED(event)) { return; }
This is trivial, but why have this useless return?
⬇️ Suggested change
- virtual void SetHelpEventPos(wxHelpEvent& WXUNUSED(event)) { return; }
+ virtual void SetHelpEventPos(wxHelpEvent& WXUNUSED(event)) { }
—
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25809)Please reset the draft status when the builds pass.
BTW, I forgot to mention this, but it's probably worth mentioning this behaviour in wxHelpEvent documentation in interface/wx/event.h. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)Modified 091825: Added a virtual function SetHelpIdAtPoint in windows.h to convert ToolBar Id in context help mode to actual pressed button Id for derived wxToolBar class
Will update the PR shortly.
***@***.***
From: VZ ***@***.***>
Sent: Thursday, September 18, 2025 12:45 PM
To: wxWidgets/wxWidgets ***@***.***>
Cc: StevenPHirshman ***@***.***>; Author ***@***.***>
[Image removed by sender.]vadz left a comment (wxWidgets/wxWidgets#25809)<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3308505948>
Please reset the draft status when the builds pass.
BTW, I forgot to mention this, but it's probably worth mentioning this behaviour in wxHelpEvent documentation in interface/wx/event.h. TIA!
—
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)***@***.***
From: VZ ***@***.***>
Sent: Thursday, September 18, 2025 12:45 PM
To: wxWidgets/wxWidgets ***@***.***>
Cc: StevenPHirshman ***@***.***>; Author ***@***.***>
Subject: Re: [wxWidgets/wxWidgets] Pass toolbar button ID in `wxContextHelpEvent` (PR #25809)
[Image removed by sender.]vadz left a comment (wxWidgets/wxWidgets#25809)<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3308505948>
Please reset the draft status when the builds pass.
BTW, I forgot to mention this, but it's probably worth mentioning this behaviour in wxHelpEvent documentation in interface/wx/event.h. TIA!
—
Reply to this email directly, view it on GitHub<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3308505948>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BNH2DLA76BSH4YFLHNGBMYL3TLOSJAVCNFSM6AAAAACG2AV5SSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMBYGUYDKOJUHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)Mac builds / wxiOS
Failed in 13 minutes and 44 seconds
***@***.***
From: VZ ***@***.***>
Sent: Thursday, September 18, 2025 12:45 PM
To: wxWidgets/wxWidgets ***@***.***>
Cc: StevenPHirshman ***@***.***>; Author ***@***.***>
Subject: Re: [wxWidgets/wxWidgets] Pass toolbar button ID in `wxContextHelpEvent` (PR #25809)
[Image removed by sender.]vadz left a comment (wxWidgets/wxWidgets#25809)<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3308505948>
Please reset the draft status when the builds pass.
BTW, I forgot to mention this, but it's probably worth mentioning this behaviour in wxHelpEvent documentation in interface/wx/event.h. TIA!
—
Reply to this email directly, view it on GitHub<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3308505948>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BNH2DLA76BSH4YFLHNGBMYL3TLOSJAVCNFSM6AAAAACG2AV5SSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMBYGUYDKOJUHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- 4d2eccd added comments to window.h and event.h
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- d01f5a4 Replace SetHelpEventPos with SetHelpIdAtPoint
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)Let me know anything else is required.
***@***.***
From: VZ ***@***.***>
Sent: Thursday, September 18, 2025 12:45 PM
To: wxWidgets/wxWidgets ***@***.***>
Cc: StevenPHirshman ***@***.***>; Author ***@***.***>
Subject: Re: [wxWidgets/wxWidgets] Pass toolbar button ID in `wxContextHelpEvent` (PR #25809)
[Image removed by sender.]vadz left a comment (wxWidgets/wxWidgets#25809)<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3308505948>
Please reset the draft status when the builds pass.
BTW, I forgot to mention this, but it's probably worth mentioning this behaviour in wxHelpEvent documentation in interface/wx/event.h. TIA!
—
Reply to this email directly, view it on GitHub<https://github.com/wxWidgets/wxWidgets/pull/25809#issuecomment-3308505948>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BNH2DLA76BSH4YFLHNGBMYL3TLOSJAVCNFSM6AAAAACG2AV5SSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMBYGUYDKOJUHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- a652daa Add files via upload
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- 67e30fa Update window.h
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- 49d211d Update tbarbase.cpp
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- 2c1c0e5 Update wincmn.cpp
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- bf2333d Update wincmn.cpp *(remove EOF lines)
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- f464248 Update tbarbase.h (EOF)
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- ba164d0 Update window.h (fix tab ln 1451)
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- 33b3c45 Update tbarbase.h (tab to space)
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 4 commits.
- a7aefb7 added comments to window.h and event.h
- 2a6b28a updated information text in event.h
- 3da5a60 updated line endings
- 3742804 Change CR/LF to LF and add terminating LF
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25809)Just for the recorded, you (or your editor) have completely reformatted all the files you have touched. You need to configure it to prevent this from happening.
Please make the changes in your fork of the repository, confirm that they look good to you by comparing them with this one and then, and only then, push them here.
Thanks in advance!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 6 commits.
- 044b3b0 Delete include/wx/window.h
- 722cdc2 Delete src/common/cshelp.cpp
- 4fbc8f0 Delete src/common/tbarbase.cpp
- 8152db1 Delete src/common/wincmn.cpp
- e59b8b2 Delete include/wx/tbarbase.h
- a76cb50 Merge branch 'StevenPHirshman'
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- 2d1858d CR/LF Mods for SetHelpIdAtPoint
You are receiving this because you are subscribed to this thread.
StevenPHirshman
@StevenPHirshman pushed 1 commit.
- b84acb2 LR?CR fix 2
You are receiving this because you are subscribed to this thread.
StevenPHirshman
Closed #25809.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
StevenPHirshman
StevenPHirshman left a comment (wxWidgets/wxWidgets#25809)Just for the recorded, you (or your editor) have completely reformatted all the files you have touched. You need to configure it to prevent this from happening.
Please make the changes in your fork of the repository, confirm that they look good to you by comparing them with this one and then, and only then, push them here.
Thanks in advance!
I updated the files. Now there are no EOL/White Space issues, but ALL compilation tests are failing now. I don't understand why that is happening.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.