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

GString support #745

Merged
merged 2 commits into from Jan 12, 2019

Conversation

Projects
None yet
4 participants
@philn
Copy link
Contributor

philn commented Dec 16, 2018

No description provided.

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 11, 2019

I suppose this one needs a rebase.

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Jan 11, 2019

Yeah, but otherwise good to go. Not sure why it was not merged before. @GuillaumeGomez ?

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 11, 2019

Never took the time to look at the monstruous diff (sorry....).

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

I did a fresh regen, and boom. 1483 errors, mostly:

   --> src/auto/list_store.rs:16:1                                                                                                                                                                  
    |                                                                                                                                                                                               
16  | / glib_wrapper! {                                                                                                                                                                             
17  | |     /// The `ListStore` object is a list model for use with a `TreeView`                                                                                                                    
18  | |     /// widget. It implements the `TreeModel` interface, and consequentialy,                                                                                                                
19  | |     /// can use all of the methods available there. It also implements the                                                                                                                  
...   |                                                                                                                                                                                             
162 | |     }                                                                                                                                                                                       
163 | | }                                                                                                                                                                                           
    | |_^ use of undeclared type or module `ptr`                                                                                                                                                    
    |                                                                                                                                                                                               
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)                                                   
                                                                                                                                                                                                    
error[E0433]: failed to resolve: use of undeclared type or module `mem`                                                                                                                             
   --> src/auto/list_store.rs:16:1                                                                                                                                                                  
    |                                                                                                                                                                                               
16  | / glib_wrapper! {                                                                                                                                                                             
17  | |     /// The `ListStore` object is a list model for use with a `TreeView`                                                                                                                    
18  | |     /// widget. It implements the `TreeModel` interface, and consequentialy,                                                                                                                
19  | |     /// can use all of the methods available there. It also implements the                                                                                                                  
...   |                                                                                                                                                                                             
162 | |     }                                                                                                                                                                                       
163 | | }                                                                                                                                                                                           
    | |_^ use of undeclared type or module `mem`                                                                                                                                                    
    |                                                                                                                                                                                               
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)                
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

Maybe you use old glib?
Its builds fine for me after rebase.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

All I done as extra is removed unused in manual app_chooser.rs, border.rs, entry_buffer.rs

-use glib_ffi;
-use gobject_ffi;
-use std::ptr;
-use std::mem;
@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

Ah yes, once again I forgot to cargo update :)

@philn philn force-pushed the philn:master branch from a66432c to c4e2065 Jan 12, 2019

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

Seems cargo build --features purge-lgpl-docs broken 😢
cc @GuillaumeGomez

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 12, 2019

Urg. Well, I'll take a look when I can. :-/

@philn philn force-pushed the philn:master branch from c4e2065 to 61b65b3 Jan 12, 2019

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

Now without the lgpl docs, should make CI happier :)

/// knows the default and last-used application for a given content
/// type. The `AppChooserWidget` provides detailed control over
/// whether the shown list of applications should include default,
/// recommended or fallback applications.

This comment has been minimized.

@EPashkin

EPashkin Jan 12, 2019

Member

Here docs still present

/// `AppChooser` is an interface that can be implemented by widgets which
/// allow the user to choose an application (typically for the purpose of
/// opening a file). The main objects that implement this interface are
/// `AppChooserWidget`, `AppChooserDialog` and `AppChooserButton`.

This comment has been minimized.

@EPashkin

EPashkin Jan 12, 2019

Member

Seems it instead with LGLP docs

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

Yes I don't know why gir generate those. The config file specifies "normal" mode... looks like another bug in gir.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

Strange but cargo build --features purge-lgpl-docs works fine for me locally

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

@philn seems you run cargo test --features embed-lgpl-docs after regen and in previous run too.
@GuillaumeGomez Oh, it standard error with inverted diff, don't worry: purge works fine.

@philn philn force-pushed the philn:master branch from 61b65b3 to fd805bf Jan 12, 2019

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

Good to go now.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

@philn Seems you forgot to update gir submodule: https://travis-ci.org/gtk-rs/gtk/jobs/478798815#L739,
but at minimum docs check now passes.

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

It's updated there, eca9399 as part of this PR

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

Submodule updated to c5498ac, but you used last master 11adffa to regen

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

As it produce same code you can just change versions.txt to c5498ac if don't want do rebase

@philn philn force-pushed the philn:master branch from fd805bf to dab11c2 Jan 12, 2019

@philn

This comment has been minimized.

Copy link
Contributor Author

philn commented Jan 12, 2019

Rebased :)

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

Thanks, clippy give errors: https://travis-ci.org/gtk-rs/gtk/jobs/478811268#L1372-L1378,
but IMHO we can ignore it in this PR as there other PRs planned.

error: casting from `*mut ffi::GtkRecentChooser` to a more-strictly-aligned pointer (`*mut gobject_ffi::GObject`)
   --> src/auto/recent_chooser.rs:329:25
    |
329 |             connect_raw(self.to_glib_none().0 as *mut _, b"notify::filter\0".as_ptr() as *const _,
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 12, 2019

Ok, CI ended, 2 travis jobs failed by clippy.
@GuillaumeGomez This PR IMHO ready for merge too

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 12, 2019

I approve. Thanks @philn!

@GuillaumeGomez GuillaumeGomez merged commit e0e18b9 into gtk-rs:master Jan 12, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

vhdirk pushed a commit to vhdirk/gtk-rs that referenced this pull request Jan 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.