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

quake: add a new option onlyone. #339

Merged
merged 1 commit into from
May 6, 2017
Merged

quake: add a new option onlyone. #339

merged 1 commit into from
May 6, 2017

Conversation

exzombie
Copy link
Contributor

@exzombie exzombie commented May 4, 2017

This option allows using a single instance of the app with multiple
screens instead of having a separate app for each screen. It works
better if there is only one instance of the quake object, i.e.

local quake = lain.util.quake({onlyone=true})

and

awful.key({ modkey, }, "z", function () quake:toggle() end),

This option allows using a single instance of the app with multiple
screens instead of having a separate app for each screen. It works
better if there is only one instance of the quake object, i.e.

  local quake = lain.util.quake({onlyone=true})

and

  awful.key({ modkey, }, "z", function () quake:toggle() end),
@lcpz lcpz merged commit a99f937 into lcpz:master May 6, 2017
@lcpz
Copy link
Owner

lcpz commented Sep 5, 2017

@liquiddandruff reported here that the option seems not working with his setup.

Since I'm not a multiscreen user, and you introduced the PR, I'd like to have your help, @exzombie.


@liquiddandruff when you spawn quake on the other screen, is the previous instance visible?

@exzombie
Copy link
Contributor Author

exzombie commented Sep 5, 2017

@liquiddandruff, do you construct your quake object inside connect_for_each_screen or globally? Option onlyone only makes sense when having a global quake object. If you make one for each screen, it can have no effect. In fact, I intended to update the wiki when this PR was merged, but forgot.

@copycat-killer, I thought that the default behaviour of spawning a new terminal for each screen was a bit strange, now I know why :) . I decided to introduce a new parameter in order not to change behaviour for existing configurations. But since using onlyone also requires moving quake to global context, would it make sense to remove the onlyone parameter and just always scan all screens? One can then choose whether to have a global or per-screen quake object, a simple documentation change.

@lcpz
Copy link
Owner

lcpz commented Sep 5, 2017

You're right, my head is elsewhere these days.

I'll remove the option and change documentation.

@liquiddandruff
Copy link

When I spawn quake on the other screen, the previous instance is still visible, yes. And I did construct the quake object for each screen instead of globally. It seems the problem has been identified; am eager for the next revision, and thanks to you both!

@exzombie
Copy link
Contributor Author

exzombie commented Sep 5, 2017

I took the liberty of updating the wiki as keybinding has to be done differently. I also removed onlyone from the table. But you still need to fix the code: on line 36, you iterate over self.screen; use nil instead. I'm glad this will be solved without a superfluous parameter :) .

lcpz pushed a commit that referenced this pull request Sep 5, 2017
This pull request was closed.
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.

3 participants