Add utilities functions #140

GuillaumeGomez · 2016-11-04T22:01:36Z

cc @gkoz @EPashkin

@jo: It should fulfill your needs.

jo · 2016-11-04T23:15:52Z

@GuillaumeGomez awesome!

How can I test this? Would it be enough to point to this branch from Cargo.toml?

Thank you so much.

GuillaumeGomez · 2016-11-04T23:20:00Z

When merged, you can just put the glib dependency like this:

glib = { git = "https://github.com/gtk-rs/glib" }

To test it, you have to make it point to my own repo:

glib = { git = "https://github.com/guillaumegomez/glib", branch = "missing_functions" }

jo · 2016-11-04T23:44:03Z

I get an error:

[jo@jolap simple-gtk-application]$ cargo run
    Updating git repository `https://github.com/guillaumegomez/glib`
    Updating git repository `https://github.com/gtk-rs/sys`
error: native library `glib` is being linked to by more than one package, and can only be linked to by one package

  glib-sys v0.3.1
  glib-sys v0.3.1 (https://github.com/gtk-rs/sys#774929a2)

jo · 2016-11-04T23:45:05Z

My Cargo.toml looks like this:

[package]
name = "simple-gtk-application"
version = "0.1.0"
authors = ["Johannes J. Schmidt <jo@die-tf.de>"]

[dependencies]
glib = { git = "https://github.com/guillaumegomez/glib", branch = "missing_functions" }
gio = "0.1.0"
hyper = "0.9.10"
serde = "0.8"
serde_json = "0.8"
serde_derive = "0.8"

[dependencies.gtk]
version = "0.1.0"
features = ["v3_12"]

jo · 2016-11-04T23:45:50Z

Should I point the other gtk-rs deps to git#master?

GuillaumeGomez · 2016-11-04T23:46:43Z

Hum... I think it's because of gtk. Just fork our gtk and update its glib dependency like I showed you and then put your gtk dependency on your repo.

Sorry to have you put through this. :-/

GuillaumeGomez · 2016-11-04T23:49:42Z

You don't use gtk? Well, what I said for gtk is still valid for gio.

jo · 2016-11-04T23:51:09Z

No worry, I am so happy you help!

Yes, I do use gtk, look at the last section of my Cargo. Do I also need to for gio?

jo · 2016-11-04T23:52:45Z

Oh, the problems continue:

[jo@jolap gtk]$ cargo build
    Updating git repository `https://github.com/gtk-rs/sys`
    ...

error[E0432]: unresolved import `glib::signal::Inhibit`
 --> src/signal.rs:8:9
  |
8 | pub use glib::signal::Inhibit;
  |         ^^^^^^^^^^^^^^^^^^^^^ no `Inhibit` in `glib::signal`

jo · 2016-11-04T23:55:11Z

Those cross repository dependencies are always a pain. I know this from node so well :(

GuillaumeGomez · 2016-11-04T23:56:25Z

Sorry, my branch wasn't up-to-date with master. Refresh and retry.

jo · 2016-11-04T23:58:33Z

How do I refresh? I deleted target directory and run cargo build again but still get the same errors

jo · 2016-11-05T00:00:58Z

This is my fork of gtk: https://github.com/jo/gtk
I did the following changes:

[jo@jolap gtk]$ git diff

diff --git a/Cargo.toml b/Cargo.toml
index f3d959c..1f39a68 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -71,8 +71,11 @@ git = "https://github.com/gtk-rs/gio"
 version = "0.1.0"

 [dependencies.glib]
-git = "https://github.com/gtk-rs/glib"
-version = "0.1.0"
+# git = "https://github.com/gtk-rs/glib"
+# version = "0.1.0"
+git = "https://github.com/guillaumegomez/glib"
+branch = "missing_functions"
+

 [dependencies.gdk]
 git = "https://github.com/gtk-rs/gdk"

GuillaumeGomez · 2016-11-05T00:02:47Z

It should be ok now, no?

jo · 2016-11-05T00:04:12Z

no :(
Maybe Cargo did not update the git checkout of your branch? How can I enforce this?

GuillaumeGomez · 2016-11-05T00:06:51Z

Well, it's not convenient to try to explain. So for now, please clone the repo locally and replace glib dependencies with glib = { path = "../glib" } (if glib and gtk are at the same level). Then from your project, replace gtk dependency with gtk = { path = "../gtk" } (if gtk is at the same level than your project).

I hope it'll make things easier while we wait this PR to get merged.

PS: for glib, don't forget to git checkout missing_functions!

jo · 2016-11-05T00:10:21Z

OK, will try this during the next days.
Now I learned how to use local dependencies, thank you so much!

GuillaumeGomez · 2016-11-05T00:11:04Z

No problem. Like I said, once merged, you won't have to trouble yourself with all this.

EPashkin · 2016-11-05T03:45:35Z

src/enums.rs

+            UserDirectory::PublicShare => ffi::G_USER_DIRECTORY_PUBLIC_SHARE,
+            UserDirectory::Templates => ffi::G_USER_DIRECTORY_TEMPLATES,
+            UserDirectory::Videos => ffi::G_USER_DIRECTORY_VIDEOS,
+            UserDirectory::NDirectories => ffi::G_USER_N_DIRECTORIES,


We really need this case in manual code?

Well, it's part of the enum so yes.

And why also don't remove (comment) in enum?

EPashkin · 2016-11-05T03:45:38Z

src/enums.rs

+            UserDirectory::NDirectories => ffi::G_USER_N_DIRECTORIES,
+        }
+    }
+}


Empty line at end pls.

EPashkin · 2016-11-05T03:47:16Z

src/utils.rs

+use translate::*;
+use UserDirectory;
+
+pub fn get_application_name() -> String {


human-readable application name. may return NULL => Option<String>

Good catch!

But it still String

EPashkin · 2016-11-05T04:01:13Z

src/utils.rs

+
+pub fn path_get_basename(file_name: &str) -> Option<String> {
+    unsafe {
+        from_glib_none(ffi::g_path_get_basename(file_name.to_glib_none().0))


Returns: a newly allocated string containing the last component of the filename. => from_glib_full?

EPashkin · 2016-11-05T04:04:22Z

src/utils.rs

+    }
+}
+
+pub fn path_get_dirname(file_name: &str) -> Option<String> {


Also from_glib_full

EPashkin · 2016-11-05T04:05:03Z

src/utils.rs

+    }
+}
+
+pub fn find_program_in_path(program: &str) -> Option<String> {


Also from_glib_full

EPashkin · 2016-11-05T04:12:11Z

src/utils.rs

+
+pub fn get_environ() -> Vec<String> {
+    unsafe {
+        FromGlibPtrContainer::from_glib_none(ffi::g_get_environ())


Also transfer full but not sure which case.

GuillaumeGomez · 2016-11-05T13:42:26Z

Updated.

GuillaumeGomez · 2016-11-05T20:30:55Z

Should be good now.

EPashkin · 2016-11-05T20:33:51Z

👍
But I still not like NDirectories in rust code.

GuillaumeGomez · 2016-11-05T20:35:10Z

Me neither but it doesn't harm to have it so...

jo · 2016-11-06T01:02:55Z

@GuillaumeGomez I give up trying to compile gtk-rs packages with local dependencies. I ended up with clones of cairo, gdk, gdk-pixbuf, gio, glib, gtk, pango and sys, used path references everywhere but I get the error:

[jo@jolap gdk]$ cargo build
error: native library `cairo` is being linked to by more than one package, and can only be linked to by one package

  cairo-sys-rs v0.3.1 (https://github.com/gtk-rs/cairo#4a61ff90)
  cairo-sys-rs v0.3.1 (file:///home/jo/github/gtk-rs/cairo/cairo-sys-rs)

This error does not tell me where it happens :/

I will wait for this pr to get landed. Its not urgent for me anyway, I just wanted to contribute my testing. Although its interesting how the libs work together.

GuillaumeGomez · 2016-11-06T09:59:31Z

I'll merge it for now. If we encounter issues, we'll just update.

jo · 2016-11-06T10:59:11Z

\o/ it works: when I now set

glib::utils::set_program_name(Some("Simple Gtk App"));

it shows the name in the menubar:

Without set_program_name it just displays a dot:

Thank you so much for your work on gtk-rs!

GuillaumeGomez · 2016-11-06T11:12:35Z

Great!

gkoz · 2016-11-08T09:39:55Z

I'd recommend removing anything that's already available via std::env, std::fs, std::net. GLib makes sense as a standard library for C, but not for Rust.

GuillaumeGomez force-pushed the missing_functions branch from 057feed to 3bc0661 Compare November 4, 2016 23:56

EPashkin reviewed Nov 5, 2016

View reviewed changes

src/enums.rs

UserDirectory::NDirectories => ffi::G_USER_N_DIRECTORIES,

}

}

}

Copy link

Member

EPashkin Nov 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty line at end pls.

EPashkin reviewed Nov 5, 2016

View reviewed changes

src/utils.rs

}

}

pub fn path_get_dirname(file_name: &str) -> Option<String> {

Copy link

Member

EPashkin Nov 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also from_glib_full

EPashkin reviewed Nov 5, 2016

View reviewed changes

src/utils.rs

}

}

pub fn find_program_in_path(program: &str) -> Option<String> {

Copy link

Member

EPashkin Nov 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also from_glib_full

EPashkin reviewed Nov 5, 2016

View reviewed changes

GuillaumeGomez force-pushed the missing_functions branch from 3bc0661 to 6b68937 Compare November 5, 2016 13:42

GuillaumeGomez added 2 commits November 5, 2016 21:29

Add utilities functions

e92a97a

remove some functions from windows platform

ecd090c

GuillaumeGomez force-pushed the missing_functions branch from 6b68937 to ecd090c Compare November 5, 2016 20:29

GuillaumeGomez merged commit 768a969 into gtk-rs:master Nov 6, 2016

GuillaumeGomez deleted the missing_functions branch November 6, 2016 09:59

GuillaumeGomez mentioned this pull request Nov 6, 2016

Add set_application_name GuillaumeGomez/process-viewer#50

Closed

Add utilities functions #140

Add utilities functions #140

Conversation

GuillaumeGomez commented Nov 4, 2016

jo commented Nov 4, 2016

GuillaumeGomez commented Nov 4, 2016

jo commented Nov 4, 2016

jo commented Nov 4, 2016

jo commented Nov 4, 2016

GuillaumeGomez commented Nov 4, 2016

GuillaumeGomez commented Nov 4, 2016

jo commented Nov 4, 2016

jo commented Nov 4, 2016

jo commented Nov 4, 2016

GuillaumeGomez commented Nov 4, 2016

jo commented Nov 4, 2016 • edited Loading

jo commented Nov 5, 2016

GuillaumeGomez commented Nov 5, 2016

jo commented Nov 5, 2016

GuillaumeGomez commented Nov 5, 2016 • edited Loading

jo commented Nov 5, 2016

GuillaumeGomez commented Nov 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Nov 5, 2016

GuillaumeGomez commented Nov 5, 2016

EPashkin commented Nov 5, 2016

GuillaumeGomez commented Nov 5, 2016

jo commented Nov 6, 2016

GuillaumeGomez commented Nov 6, 2016

jo commented Nov 6, 2016

GuillaumeGomez commented Nov 6, 2016

gkoz commented Nov 8, 2016

jo commented Nov 4, 2016 •

edited

Loading

GuillaumeGomez commented Nov 5, 2016 •

edited

Loading