fireplace icon indicating copy to clipboard operation
fireplace copied to clipboard

Close view command doesn't work.

Open Timidger opened this issue 8 years ago • 20 comments

I am using this configuration, and I can't get the quit to work (I expect it to be Super+Shift+Q, but it does not do that.).

All the other commands seem to work.

Timidger avatar Feb 24 '17 03:02 Timidger

Are you sure the keys aren't grabbed by a different application, like a host window manager? Are you running fireplace in XWayland or by itself?

IntrepidPig avatar Feb 24 '17 04:02 IntrepidPig

I ran it by itself, the keys are definitely not being grabbed because e.g my terminal writes "Q" instead of quitting the application.

Timidger avatar Feb 24 '17 04:02 Timidger

Running your config file right now and it works fine... Can you share some logs please?

Drakulix avatar Feb 24 '17 16:02 Drakulix

Here's the log, it doesn't show much so if there's a better way to enable debugging please tell me and I can update it.

All I did was open a terminal, tried to close it a few times, and then quit the wm.

Timidger avatar Feb 24 '17 17:02 Timidger

Indeed, it does not show much, I never though I would have to actually log keys for debugging. Could you try to add this line: info!(slog_scope::logger(), "View bindings: {:?}", config.view.keys); at: https://github.com/Drakulix/fireplace/blob/master/fireplace/src/main.rs#L70

Like this we can figure out, if its the config file, that is not parsed correctly or if it is actually no detecting your key presses correctly (which I highly doubt, if you can launch a terminal and quit fireplace correctly).

Also this revealed another bug. I forget to fix the default config for the new screenshot feature, but this is not relevant for this issue, as I am running your exact config just fine right now.

Drakulix avatar Feb 24 '17 18:02 Drakulix

I tried running both the latest release and the release of the one I downloaded in binary form (v.3.1.0) and on both of them I got an error at runtime reading the configuration file instead:

thread 'main' panicked at 'Malformed config file: Message("unknown field `background`, expected `gaps`"
, Some(Pos { marker: Marker { index: 620, line: 20, col: 12 }, path: "outputs.generic.ui" }))', /checko
ut/src/libcore/result.rs:860                                                                          
stack backtrace:
   1:     0x5602500e9da9 - std::sys::imp::backtrace::tracing::imp::write::hbb14611794d3841b
                        at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:42
   2:     0x5602500ee85e - std::panicking::default_hook::{{closure}}::h6ed906c7818ac88c
                        at /checkout/src/libstd/panicking.rs:351
   3:     0x5602500ee464 - std::panicking::default_hook::h23eeafbf7c1c05c3
                        at /checkout/src/libstd/panicking.rs:367
   4:     0x5602500eecbb - std::panicking::rust_panic_with_hook::hd0067971b6d1240e
                        at /checkout/src/libstd/panicking.rs:545
   5:     0x5602500eeb44 - std::panicking::begin_panic::h1fd1f10a3de8f902
                        at /checkout/src/libstd/panicking.rs:507
   6:     0x5602500eeab9 - std::panicking::begin_panic_fmt::haa043917b5d6f21b
                        at /checkout/src/libstd/panicking.rs:491
   7:     0x5602500eea47 - rust_begin_unwind
                        at /checkout/src/libstd/panicking.rs:467
   8:     0x560250119d9d - core::panicking::panic_fmt::he9c7f335d160b59d
                        at /checkout/src/libcore/panicking.rs:69
   9:     0x56024fed13e2 - core::result::unwrap_failed::h333207854275ca65
                        at /checkout/src/libcore/macros.rs:29
  10:     0x56024feb86cf - <core::result::Result<T, E>>::expect::hb7a03301b39bafb8
                        at /checkout/src/libcore/result.rs:761
  11:     0x56024ff49810 - fireplace::try_config_locations::h6c4a76ac974d4112
                        at /home/timidger/src/fireplace/fireplace/src/main.rs:37
  12:     0x56024ff4a2d6 - fireplace::main::h3f904b7c1bf2a8fa
                        at /home/timidger/src/fireplace/fireplace/src/main.rs:66
  13:     0x5602500f5bea - __rust_maybe_catch_panic
                        at /checkout/src/libpanic_unwind/lib.rs:98
  14:     0x5602500ef466 - std::rt::lang_start::hb7fc7ec87b663023
                        at /checkout/src/libstd/panicking.rs:429
                        at /checkout/src/libstd/panic.rs:361
                        at /checkout/src/libstd/rt.rs:57
  15:     0x56024ff56842 - main
  16:     0x7f6c38a70290 - __libc_start_main
  17:     0x56024fdaef69 - _start
  18:                0x0 - <unknown>

EDIT:

I compared the hashes of the binaries, and they are not the same.

I also noticed you don't have a --version flag, that would be really helpful when debugging issues especially when it could be a user is on an older version.

For Way Cooler, we have the git version compiled into the binary and shown when we pass --version into the binary. That allows us to run the exact same version that the user is having trouble with. You might consider adding something like that for fireplace.

Timidger avatar Feb 24 '17 19:02 Timidger

For Way Cooler, we have the git version compiled into the binary and shown when we pass --version into the binary. That allows us to run the exact same version that the user is having trouble with. You might consider adding something like that for fireplace.

Seems like a good idea.

The Issue you are getting happens, when you build fireplace without the ui feature. The binary version from the releases page has the feature build in for sure (I just tried it) and does not throw that error you are experiencing. I am questioning, if you really did try that version.

Also could you please give me the git tag of your current repository (git rev-parse HEAD), so I can test with that version?

Drakulix avatar Feb 24 '17 20:02 Drakulix

Here is the updated log, I fixed my build so it will run now. I was disabling all the features, because I don't have the static wlc dependencies in the right place and didn't want to mess with that. So I had to run the very silly looking line --no-default-features --features=ui

log ref: 3c637e9d (tag v.3.1.0)

Timidger avatar Feb 26 '17 18:02 Timidger

I was disabling all the features, because I don't have the static wlc dependencies in the right place and didn't want to mess with that. So I had to run the very silly looking line --no-default-features --features=ui

Just so you know, this is also documented here :wink:
https://github.com/Drakulix/fireplace/tree/master/fireplace#building

Thanks for reporting back, this continues to be a very interesting bug, that I would really like to have resolved and better documentated. So the log shows the config file is parsed correctly and that closing really should work as expected.

Could you also please enable the file option in the configuration file at logging: to let fireplace write its own configuration file? That would also include the Debug log level, which will tell us, if the key combination is not only correctly passed, but also detected, when you try it to use it.

Drakulix avatar Feb 26 '17 20:02 Drakulix

Just so you know, this is also documented here 😉

Yeah, that's where I found the option. I missed it the first time. I think I'm just too used to my own workflow for building my compositor, and my brain is too stubborn to do it any other way ;).

Could you also please enable the file option in the configuration file at logging: to let fireplace write its own configuration file? That would also include the Debug log level, which will tell us, if the key combination is not only correctly passed, but also detected, when you try it to use it.

Sure, here you go (sorry for the dumb file extension, github has silly restrictions)

Timidger avatar Feb 26 '17 20:02 Timidger

Alright this does now get a bit weird.

As we can clearly see in the first line you config is parsed correctly. You can also see how other keys activate in your configuration DEBG Matching KeyPattern. But your view.close combination is not displayed and of course also not executed.

That means that wlc's keyboard_key hook is not getting exactly the information from the configuration file. I suspect, that maybe another modifier is pressed as well or something similar.

The line that should match the your combination is the following: https://github.com/Drakulix/fireplace/blob/master/fireplace_lib/src/handlers/keyboard.rs#L129

I would ask you one more time to add another line below: debug!("Testing key combination: {:?}", pattern);

So we can see what exactly it is what fireplace receives from wlc.

Drakulix avatar Feb 26 '17 21:02 Drakulix

Trying to add that line causes this compilation error message:

error: no method named `log` found for type `&'static str` in the current scope
   --> /home/timidger/src/fireplace/fireplace_lib/src/handlers/keyboard.rs:130:9
    |
130 |         debug!("Testing key combination: {:?}", pattern);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `handlers::keyboard::KeyPattern: std::fmt::Display` is not satisfied
   --> /home/timidger/src/fireplace/fireplace_lib/src/handlers/keyboard.rs:130:49
    |
130 |         debug!("Testing key combination: {:?}", pattern);
    |                                                 ^^^^^^^ the trait `std::fmt::Display` is not imple
mented for `handlers::keyboard::KeyPattern`                                                            
    |
    = note: `handlers::keyboard::KeyPattern` cannot be formatted with the default formatter; try using `
:?` instead if you are using a format string                                                           
    = note: required by `std::fmt::Display::fmt`

error: aborting due to 2 previous errors

error: Could not compile `fireplace_lib`.

I find it strange it's saying it doesn't implemen Display::fmt cause I'm using "{?}", I assume it's just a weird hiccup due to the first error.

Timidger avatar Feb 26 '17 22:02 Timidger

Ah yes, forgot the logger, sorry debug!(self.logger, "Testing key combination: {:?}", pattern);

Drakulix avatar Feb 26 '17 23:02 Drakulix

Ah thanks. Not used to this logging system. Tried to switch Way Cooler to it, but a bit harder when everything is in lazy_static's ;).

Up-to-date log

Timidger avatar Feb 27 '17 00:02 Timidger

Just as a small update--I can get it to work if I make it set to close on "Logo" + "Left" (i.e left arrow). So it's something to do with how "Q" is parsed.

Just a note -- we had a similar issue like this in Way Cooler just after release. For us, it came down to us using CString incorrectly. We were getting a pointer to a string passed to us from C, but because we did it all in one line the pointer was immediately invalid as soon as we had access to it. This led to undefined behaviour, because we no longer owned the memory to it.

I don't know if there is something similar happening here, but I thought I'd share that just in case it helps.

Timidger avatar Feb 27 '17 22:02 Timidger

I don't think that is the problem, if you look at the log file, you see, that your Q key is indeed detected, just as X, not as Q. Feb 26 19:02:54.362 DEBG Testing key combination: KeyPattern { state: Pressed, modifiers: Shift | Logo, key: X }, instance: KeyboardHandler, Library: Fireplace, version: 3.1.0

That mapping is established directly with uinput keysyms: https://github.com/Drakulix/wlc.rs/blob/master/src/input/consts.rs

I suspect, that this is more of a configuration issue on your side. Could you maybe try to set your keyboard layout explicitly?

Drakulix avatar Feb 28 '17 16:02 Drakulix

Ah....that makes sense. I use dvorak, and where the "Q" key is is where the "X" key is. That would be the "problem".

If this is expected behaviour, it might need to be documented in an easy to find place.

EDIT: ~It still doesn't work even when I press "X".~ It does work if I set the keyboard back to qwerty (e.g unset XKB_DEFAULT_LAYOUT)

EDIT x2: My bad, it does work, just not as I expected. I press where the "Q" should be on the keyboard with QWERTY and it works. So this behaves exactly like how i3 works.

Timidger avatar Feb 28 '17 17:02 Timidger

I am not sure, where your layout should be set. It did work out-of-the-box in your terminal, but not in fireplace. I wonder if there is actually a configuration file somewhere, I should use to read out to set the correct layout at start. Anyway I am leaving this open to add some more documentation.

Drakulix avatar Feb 28 '17 17:02 Drakulix

EDIT x2: My bad, it does work, just not as I expected. I press where the "Q" should be on the keyboard with QWERTY and it works. So this behaves exactly like how i3 works.

What would you suggest on how this could be better documentated?

Drakulix avatar Feb 28 '17 21:02 Drakulix

Hm, I would just say that keybindings are bound to the key specified by the system default keyboard (and NOT what is set through xkb).

If that doesn't sound sufficient, perhaps just copy what i3 does (if they do anything, I can't remember if that is in the official documentation).

Timidger avatar Mar 01 '17 03:03 Timidger