Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shorter "super" parameters for virtual override methods #136

Open
5k3105 opened this issue Jan 16, 2025 · 4 comments
Open

Shorter "super" parameters for virtual override methods #136

5k3105 opened this issue Jan 16, 2025 · 4 comments
Labels

Comments

@5k3105
Copy link

5k3105 commented Jan 16, 2025

Using QTreeView OnCurrentChanged, to make it work is a little more verbose than my previous code:

view.OnCurrentChanged(view.current_changed)
view.OnDoubleClicked(view.navpane_item_clicked)

func (view *NavigationPane) current_changed(super func(current *qt.QModelIndex, previous *qt.QModelIndex), current *qt.QModelIndex, previous *qt.QModelIndex) {
	
}

func (view *NavigationPane) navpane_item_clicked(index *qt.QModelIndex) {

}

/// old version
func (view *NavigationPane) _current_changed(current *qt.QModelIndex, previous *qt.QModelIndex) {

}

However, OnDoubleClicked is the same as previous code. Is there a way to make it use just the two parameters?

I noticed in the examples its also like this

w.OnPaintEvent(func(super func(ev *qt.QPaintEvent), ev *qt.QPaintEvent) {
	...
})

Basically, is there away to remove the need to specify a func inside a func (super func) ?

@mappu mappu added the wishlist label Jan 17, 2025
@mappu
Copy link
Owner

mappu commented Jan 17, 2025

PaintEvent is not a signal/slot - actually it's a C++ virtual method override.

In C++, you should usually call the parent function inside your callback.

In therecipe/qt this was exposed as QWidget.PaintEventDefault(), but I think new Qt users didn't know this well.

Because Miqt puts it as a parameter, there can be a linter warning if it is not used. But I agree that it's inconvenient to type.

  • One idea is to switch Miqt to the therecipe/qt design
  • Another idea is to add a short typedef for this long func type. Maybe that can be a good middle-ground solution

@mappu mappu changed the title Shorter signal/slot parameters (?) Shorter "super" parameters for virtual override methods Jan 17, 2025
@5k3105
Copy link
Author

5k3105 commented Jan 17, 2025

For CurrentChanged, it just needs I think 2 parameters

void QTreeView::currentChanged(const QModelIndex &current, const QModelIndex &previous)

For PaintEvent, it is a custom type/class

void QWidget::paintEvent(QPaintEvent *event)

Forgive me for not understanding nuances of cpp, qt or even go....

@5k3105
Copy link
Author

5k3105 commented Jan 17, 2025

I'd also like to mention how nice it is having a new qt library for go. It compiles as fast as go and current conversions (moving from therecipe) have been very quick. I had also been using therecepie with GO111MODULE=off and now I can use go modules :) Thanks!

@arnetheduck
Copy link
Contributor

this was exposed as QWidget.PaintEventDefault()

Using Default might not be a good long-term idea since there might be multiple implementations/overrides when considering multiple levels of inheritance, so it probably makes sense to "mimic" the C++ style here (this->QWidget::paintEvent should probably be something like QWidget_PaintEvent alongside the normal PaintEvent function) - this is the case for the qt_metacall family of functions for example and probably can happen with QAbstractItemModel chains.

Putting it as parameter as it's done now also looks like it introduces a tiny bit of overhead for every call because the "this" parameter must be captured in a closure environment for the callback - not sure if this is significant or not, but it happens even if the "super" call ends up not being called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants