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

Espanso: Fix broken module to be compatible with Espanso version 2.x #4066

Merged
merged 10 commits into from Jun 9, 2023

Conversation

liyangau
Copy link
Contributor

@liyangau liyangau commented Jun 7, 2023

Description

Espanso module does not work with 2.x version. In the new version configuration and matches are put into different folders as below.

config/
  default.yml
match/
  base.yml

The updated module supports creating multiple files for both config and matches. Users have the full flexibility to create multiple files for different apps.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • 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.

This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and
`match` folder.
This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and `match` folder.

Add descriptions
Copy link
Member

@ncfavier ncfavier left a comment

Choose a reason for hiding this comment

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

This needs

  • a mkRemovedOptionModule for the removed settings option
  • an assertion that versionAtLeast cfg.package.version "2"

type = yaml.type;
default = { matches = [ ]; };
default = { default = { }; };
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am following previous config here. I guess there is no harm to set an empty default when the users do not provide anything.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry, I was unclear. I meant: do we need a default that's not just {}? I.e., why the default inside the default?

Copy link
Contributor Author

@liyangau liyangau Jun 8, 2023

Choose a reason for hiding this comment

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

I use default = { }; inside default is to create an empty ~/.config/espanso/config/default.yml when it is not set.

Without it, Espanso will throw below error.

[ERROR] unable to load config

Caused by:
    missing config directory


matches = mkOption {
type = yaml.type;
default = { default.matches = [ ]; };
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above

modules/services/espanso.nix Outdated Show resolved Hide resolved
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-override-service-espanso-module-for-home-manager/28551/11

Copy link

@bobvanderlinden bobvanderlinden left a comment

Choose a reason for hiding this comment

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

Haven't tried to run this, but code looks good overal 👍 small suggestion.

modules/services/espanso.nix Outdated Show resolved Hide resolved
@liyangau
Copy link
Contributor Author

liyangau commented Jun 8, 2023

Looks like pkgs.espanso does not passthru.test version. Shall I remove the checks of versionAtLeast espansoVersion "2"; to move this forward or is there any other methods we can check this?

@ncfavier
Copy link
Member

ncfavier commented Jun 8, 2023

The stubs don't have versions by default. I'm working on it.

@ncfavier
Copy link
Member

ncfavier commented Jun 8, 2023

Should be fixed now if you rebase on master.

modules/services/espanso.nix Outdated Show resolved Hide resolved
modules/services/espanso.nix Outdated Show resolved Hide resolved
liyangau and others added 2 commits June 9, 2023 19:30
Co-authored-by: Naïm Favier <n@monade.li>
@ncfavier ncfavier linked an issue Jun 9, 2023 that may be closed by this pull request
2 tasks
@ncfavier ncfavier merged commit 1e5d741 into nix-community:master Jun 9, 2023
3 checks passed
ncfavier pushed a commit to ncfavier/home-manager that referenced this pull request Jun 12, 2023
…ix-community#4066)

* Fix espanso module to work with 2.x version

* espanso: fix espanso module

This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and
`match` folder.

* Espanso: fix espanso module

This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and `match` folder.

Add descriptions

* Add versionAtLeast and mkRemovedOptionModule

* Correct maintainers list

* remove config key from example

* format basic-configuration.nix

* Update modules/services/espanso.nix

Co-authored-by: Naïm Favier <n@monade.li>

* fix maintainers list

---------

Co-authored-by: Naïm Favier <n@monade.li>
(cherry picked from commit 1e5d741)
@ncfavier ncfavier mentioned this pull request Jun 12, 2023
6 tasks
ncfavier pushed a commit to ncfavier/home-manager that referenced this pull request Jun 12, 2023
…ix-community#4066)

* Fix espanso module to work with 2.x version

* espanso: fix espanso module

This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and
`match` folder.

* Espanso: fix espanso module

This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and `match` folder.

Add descriptions

* Add versionAtLeast and mkRemovedOptionModule

* Correct maintainers list

* remove config key from example

* format basic-configuration.nix

* Update modules/services/espanso.nix

Co-authored-by: Naïm Favier <n@monade.li>

* fix maintainers list

---------

Co-authored-by: Naïm Favier <n@monade.li>
(cherry picked from commit 1e5d741)
ncfavier added a commit that referenced this pull request Jun 12, 2023
* tests: `--show-trace` in CI (#4070)

(cherry picked from commit f889ec0)

* tests/stubs: inherit default versions from pkgs (#4069)

* tests/stubs: inherit default versions from pkgs

* tests/browserpass: temporarily disable on darwin

The package currently fails to evaluate on darwin due to a nixpkgs
problem: NixOS/nixpkgs#236258 (comment)

(cherry picked from commit 69bdd6d)

* Espanso: Fix broken module to be compatible with Espanso version 2.x (#4066)

* Fix espanso module to work with 2.x version

* espanso: fix espanso module

This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and
`match` folder.

* Espanso: fix espanso module

This module is currently broken. It does not create `config` and `match` folders which are required by espanso 2.x version.

This PR fixed this issue and support creating multiple files under `config` and `match` folder.

Add descriptions

* Add versionAtLeast and mkRemovedOptionModule

* Correct maintainers list

* remove config key from example

* format basic-configuration.nix

* Update modules/services/espanso.nix

Co-authored-by: Naïm Favier <n@monade.li>

* fix maintainers list

---------

Co-authored-by: Naïm Favier <n@monade.li>
(cherry picked from commit 1e5d741)

---------

Co-authored-by: Li Yang <71299093+liyangau@users.noreply.github.com>
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.

bug: espanso >=2 (now in nixpkgs) requires config update
4 participants