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

sway: add config.bindkeysToCode #1289

Closed

Conversation

rvolosatovs
Copy link
Contributor

Description

Do not use --to-code by default in bindsym, let user configure that behavior manually via config.bindkeysToCode in sway module.

Refs #1229 (comment)

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

tests fail with:

hash mismatch in fixed-output derivation '/nix/store/sd0pnznvqy3l9k773anr14kv5imchvg8-delta-0.0.16-vendor.tar.gz':
  wanted: sha256:1zmk70hccrxn1gdr1bksnvh6sa2h4518s0ni8k2ihxi4ld1ch5p2
  got:    sha256:0hah0qfgnl4w2h0djyh4xx1jks5dkzwin01qw001dqiasl60prn2
cannot build derivation '/nix/store/a9j70nsz3x37mkakcqjw60rl1zs4s4p6-delta-0.0.16.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/7z4qg944anpk1hfjfaxx9zv13c9bpinx-git-expected.conf.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/75y9mfswm5x82lrj28dii5yg6lyzmg68-hm_gitconfig.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/bc22arqm3vjb37jqdb2lag30p2rj9ajv-home-manager-files.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/2zzxyhscan5n2zgjqflbhkcc5vrcjvqh-nmt-test-script-git-with-most-options.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/zrdw6v8dixg1gap8scw11i8ccjh79hgv-nmt-report-git-with-most-options.drv': 1 dependencies couldn't be built
error: build of '/nix/store/zrdw6v8dixg1gap8scw11i8ccjh79hgv-nmt-report-git-with-most-options.drv' failed
  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@rvolosatovs rvolosatovs requested a review from rycee as a code owner May 30, 2020 12:07
@rvolosatovs
Copy link
Contributor Author

cc @dermetfan for testing

Copy link
Collaborator

@dermetfan dermetfan left a comment

Choose a reason for hiding this comment

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

I checked out your branch and ran the tests. I only get one failed test about formatting that can be solved with ./format. There is no hash mismatch error for me...

Patch produced by ./format
commit 079857636f08872c0c13ce198d2de717d1cc9f56 (HEAD -> fix/sway-bindkeys)
Author: Robin Stumm <serverkorken@gmail.com>
Date:   Sun May 31 15:07:05 2020 +0200

    sway: format

diff --git a/modules/services/window-managers/i3-sway/lib/functions.nix b/modules/services/window-managers/i3-sway/lib/functions.nix
index 4577d09..ea2938e 100644
--- a/modules/services/window-managers/i3-sway/lib/functions.nix
+++ b/modules/services/window-managers/i3-sway/lib/functions.nix
@@ -31,7 +31,7 @@ rec {

   modeStr = name: keybindings: ''
     mode "${name}" {
-    ${keybindingsStr { inherit keybindings; } }
+    ${keybindingsStr { inherit keybindings; }}
     }
   '';

diff --git a/modules/services/window-managers/i3-sway/sway.nix b/modules/services/window-managers/i3-sway/sway.nix
index 5c5bfa3..b000c42 100644
--- a/modules/services/window-managers/i3-sway/sway.nix
+++ b/modules/services/window-managers/i3-sway/sway.nix
@@ -268,9 +268,10 @@ let
       client.placeholder ${colorSetStr colors.placeholder}
       client.background ${colors.background}

-      ${keybindingsStr {
+      ${keybindingsStr {
         inherit keybindings;
-        bindsymArgs = lib.optionalString (cfg.config.bindkeysToCode) "--to-code";
+        bindsymArgs =
+          lib.optionalString (cfg.config.bindkeysToCode) "--to-code";
       }}
       ${keycodebindingsStr keycodebindings}
       ${concatStringsSep "\n" (mapAttrsToList inputStr input)}

@rvolosatovs
Copy link
Contributor Author

@dermetfan can you confirm that this patch fixes your issue?

@dermetfan
Copy link
Collaborator

Yes, thanks for the quick reaction! No idea why Travis fails now...

@rycee
Copy link
Member

rycee commented Jun 2, 2020

Beside the comment I added above this looks OK to me.

The test failure is due to SRI hashes introduced in Nixpkgs, these are unsupported in the Nix version used by Travis CI. I believe the SRI hashes are being removed from Nixpkgs master which should clear up the errors once the removals hit the unstable channel.

Do not use `--to-code` by default in `bindsym`
@rvolosatovs rvolosatovs requested a review from rycee June 3, 2020 09:17
dermetfan pushed a commit to dermetfan/home-manager that referenced this pull request Jun 3, 2020
Do not use `--to-code` by default in `bindsym`

PR nix-community#1289
rycee pushed a commit that referenced this pull request Jun 4, 2020
Do not use `--to-code` by default in `bindsym`.

PR #1289
@rycee
Copy link
Member

rycee commented Jun 4, 2020

Thanks! Rebased to master in 8574817.

@rycee rycee closed this Jun 4, 2020
colemickens pushed a commit to colemickens/home-manager that referenced this pull request Jun 24, 2020
Do not use `--to-code` by default in `bindsym`.

PR nix-community#1289
colemickens pushed a commit to colemickens/home-manager that referenced this pull request Jun 27, 2020
Do not use `--to-code` by default in `bindsym`.

PR nix-community#1289
@rvolosatovs rvolosatovs deleted the fix/sway-bindkeys branch July 20, 2020 10:58
colemickens pushed a commit to colemickens/home-manager that referenced this pull request Jul 20, 2020
Do not use `--to-code` by default in `bindsym`.

PR nix-community#1289
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

3 participants