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

QObjectBox struct is redundant? #170

Open
ratijas opened this issue Jul 10, 2021 · 2 comments · May be fixed by #178
Open

QObjectBox struct is redundant? #170

ratijas opened this issue Jul 10, 2021 · 2 comments · May be fixed by #178

Comments

@ratijas
Copy link
Collaborator

ratijas commented Jul 10, 2021

QObjectBox is only ever used for the old kefia example in a form of a patch.

Seems like the functionality was superseded by QObjectPinned, except there is no lifetime, and content is owned. I wonder, if there is any use for it?

Searching 73 files for "QObjectBox"

/qt/qmetaobject-rs/examples/kefia/0002-Port-to-qmetaobject-rs.patch:
   91   
   92  -    let qpckgs = QPackages::new(Packages {
   93: +    let qpckgs = QObjectBox::new(Packages {
   94           vec: gathered,
   95           list: list,

/qt/qmetaobject-rs/qmetaobject/src/lib.rs:
  214      pub use crate::{
  215          qml_register_type, qrc, qt_base_class, qt_method, qt_plugin, qt_property, qt_signal,
  216:         QAbstractListModel, QByteArray, QColor, QDate, QDateTime, QModelIndex, QObject, QObjectBox,
  217          QPointer, QQmlExtensionPlugin, QQuickItem, QQuickView, QRectF, QString, QTime, QVariant,
  218          QmlEngine,
  ...
  543  
  544  /// A wrapper around RefCell<T>, whose content cannot be move in memory
  545: pub struct QObjectBox<T: QObject + ?Sized>(Box<RefCell<T>>);
  546  
  547: impl<T: QObject> QObjectBox<T> {
  548      pub fn new(obj: T) -> Self {
  549:         QObjectBox(Box::new(RefCell::new(obj)))
  550      }
  551  }
  552  
  553: impl<T: QObject + Default> Default for QObjectBox<T> {
  554      fn default() -> Self {
  555          Self::new(Default::default())
  ...
  557  }
  558  
  559: impl<T: QObject + ?Sized> QObjectBox<T> {
  560      pub fn pinned(&self) -> QObjectPinned<T> {
  561          unsafe { QObjectPinned::new(&self.0) }
@ogoffart
Copy link
Member

I don't remember, but you're probably right.

Note that also QObjectPinned should be changed to use std::Pin now, that did not exist at the time this API was developed.

@ratijas
Copy link
Collaborator Author

ratijas commented Jul 12, 2021

Note that also QObjectPinned should be changed to use std::Pin now, that did not exist at the time this API was developed.

Yes, I'll get to it someday too. Before that, I'd want to clean up more things.

ratijas added a commit to ratijas/qmetaobject-rs that referenced this issue Jul 15, 2021
Goodbye, you have to go. We've never been close friends anyway.

Kefia example got a fresh look, although still using Quick Controls 1. I
unironically had to find ancient ruins of Yaourt repo, build and
install it, just to check the format of its -Qe output, because fields
were wrongly named and that got me confused. Expac replacement is
changes behavior to list all packages from all repos instead of only
locally installed ones, but that's something not easy to workaround,
and its better than having forever-obsolete and forgotten dependency.

[ChangeLog][Breaking Change] QObjectBox was removed.

Fixes woboq#170
ratijas added a commit to ratijas/qmetaobject-rs that referenced this issue Jul 15, 2021
Goodbye, you have to go. We've never been close friends anyway.

Kefia example got a fresh look, although still using Quick Controls 1.x.
I unironically had to find ancient ruins of Yaourt repo, build and
install it, just to check the format of its -Qe output, because fields
were wrongly named and that got me confused. Expac replacement is
changes behavior to list all packages from all repos instead of only
locally installed ones, but that's something not trivial to workaround,
and its better than having forever-obsolete and forgotten dependency.

[ChangeLog][Breaking Change] QObjectBox was removed.

Fixes woboq#170
@ratijas ratijas linked a pull request Jul 15, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants