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

Limit lifetime of cloned objects #16

Merged
merged 1 commit into from
Oct 9, 2018
Merged

Conversation

tathanhdinh
Copy link
Contributor

@tathanhdinh tathanhdinh commented Oct 5, 2018

Hello all,

There are some cloned objects which may not need to live as long as cloning object, for example:

glide/src/ui_context.rs

Lines 50 to 54 in c019573

let pause_button = gtk::Button::new();
let pause_actionable = pause_button.clone().upcast::<gtk::Actionable>();
pause_actionable.set_action_name("app.pause");

can be rewritten as:

let pause_button = {
  let button = gtk::Button::new();
  button.clone().upcast::<gtk::Actionable>().set_action_name("app.pause");
  button
};

...cloned pause_button needn't to live here..

It's just a master of taste, and it does not contribute anything new so feel free to reject if you don't like.

Many thanks for any comment.

@philn
Copy link
Owner

philn commented Oct 8, 2018

Oh nice, a builder file :) It doesn't seem to be used though?

@tathanhdinh
Copy link
Contributor Author

tathanhdinh commented Oct 8, 2018

@philn Sorry, sorry, this UI file is not used yet. It should not be included in the PR.
I'm push it by chance just hours ago, it's rather a WIP where I try to use GtkBuilder to separate UI from code of glide.
I will move this file into another branch.

@philn philn merged commit 29cdfe0 into philn:master Oct 9, 2018
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 this pull request may close these issues.

None yet

2 participants