Skip to content

thunderbird: support setting search engines#5697

Merged
teto merged 2 commits intonix-community:masterfrom
kira-bruneau:thunderbird-search-engines
Oct 14, 2024
Merged

thunderbird: support setting search engines#5697
teto merged 2 commits intonix-community:masterfrom
kira-bruneau:thunderbird-search-engines

Conversation

@kira-bruneau
Copy link
Copy Markdown
Contributor

@kira-bruneau kira-bruneau commented Aug 1, 2024

Description

  • Splits firefox search engine configuration into a separate submodule so it can be reused in thunderbird
  • Removes myself maintainer for all of firefox, and adds myself as a maintainer for just the search engine submodule

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.

    Submodule already tested in firefox-profile-search tests.

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

Maintainer CC

Firefox:
@rycee @brckd

Thunderbird:
@d-dervishi

@github-actions github-actions bot added the mail HM email accounts, thunderbird, alot, notmuch, msmtp, meli... label Aug 1, 2024
@kira-bruneau kira-bruneau force-pushed the thunderbird-search-engines branch 2 times, most recently from 0cbe471 to 85bcdc2 Compare August 1, 2024 05:45
@kira-bruneau
Copy link
Copy Markdown
Contributor Author

kira-bruneau commented Aug 1, 2024

I was also thinking of splitting out generating user.js & chrome from firefox so changes there would also be shared in thunderbird (see #3808, #5077, #5670), but decided to keep this focused on search for now to simplify the review.

@kira-bruneau kira-bruneau force-pushed the thunderbird-search-engines branch from 85bcdc2 to 10b321f Compare August 1, 2024 19:47
@kira-bruneau
Copy link
Copy Markdown
Contributor Author

I also just pushed a change that also organizes the firefox tests by submodule.

@kira-bruneau kira-bruneau force-pushed the thunderbird-search-engines branch from 10b321f to 7dbd977 Compare August 1, 2024 19:49
@kira-bruneau
Copy link
Copy Markdown
Contributor Author

kira-bruneau commented Aug 1, 2024

Actually, I just decided to split off test refactoring into it's own PR, because the change is easier to review separately: #5698

@kira-bruneau kira-bruneau force-pushed the thunderbird-search-engines branch 2 times, most recently from 8ff99b4 to b646398 Compare August 1, 2024 20:19
@github-actions github-actions bot added mail HM email accounts, thunderbird, alot, notmuch, msmtp, meli... and removed mail HM email accounts, thunderbird, alot, notmuch, msmtp, meli... labels Aug 1, 2024
Copy link
Copy Markdown
Contributor

@brckd brckd left a comment

Choose a reason for hiding this comment

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

Thanks @kira-bruneau! No issues from my side, since it doesn't affect the functionality of the Firefox module. The formatting only conflicts with #5685 so far, which would become your responsibility when you become the maintainer of this module.

Comment on lines +146 to +151
enable = mkOption {
type = with types; bool;
default = config.default != null || config.privateDefault != null
|| config.order != [ ] || config.engines != { };
internal = true;
};
Copy link
Copy Markdown
Contributor Author

@kira-bruneau kira-bruneau Aug 3, 2024

Choose a reason for hiding this comment

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

I added this internal enable option to make the implicit enable behaviour reusable, but I think ideally users should be required to set enable explicitly. I just didn't want to introduce any breaking changes here.

I plan on folding the force option into enable in a follow-up PR.

teto pushed a commit that referenced this pull request Oct 7, 2024
Split off from #5697, organizes firefox tests by submodule.

This is intended to match directory structure setup for the new search submodule.
Mikilio pushed a commit to Mikilio/home-manager that referenced this pull request Oct 11, 2024
Split off from nix-community#5697, organizes firefox tests by submodule.

This is intended to match directory structure setup for the new search submodule.
@kira-bruneau kira-bruneau force-pushed the thunderbird-search-engines branch from 67b1242 to d58208f Compare October 12, 2024 13:07
@teto teto merged commit e1aec54 into nix-community:master Oct 14, 2024
@khaneliman
Copy link
Copy Markdown
Collaborator

khaneliman commented Oct 14, 2024

Getting this after this commit. Both files referenced with different definitions are the firefox module in home-manager.

error: The option `home-manager.users.khaneliman.home.file.".mozilla/firefox/khaneliman/search.json.mozlz4".enable' has conflicting definition values:
       - In `/nix/store/i9vkk0yd40vsvn73a1rfi60p2p0dhc8m-source/modules/programs/firefox.nix': true
       - In `/nix/store/i9vkk0yd40vsvn73a1rfi60p2p0dhc8m-source/modules/programs/firefox.nix': false

@teto
Copy link
Copy Markdown
Collaborator

teto commented Oct 14, 2024

before merging, I managed to rebuild firefox with custom search engines and started thunderbird (that I dont use ) fine hence the merge. I apaprently missed some stuff.

@khaneliman
Copy link
Copy Markdown
Collaborator

Hmm.. I can double check my config.. but I didn't spend too much time looking originally because both files referenced were an internal home-manager module.

@khaneliman
Copy link
Copy Markdown
Collaborator

khaneliman commented Oct 17, 2024

Appears to be issue with multiple profiles sharing a path. One profile doesn't set the search option, the other uses it. This conflict is thrown.

      profiles = {
        "dev-edition-default" = {
          id = 0;
          path = "${config.${namespace}.user.name}";
        };

        ${config.${namespace}.user.name} = {
          inherit (cfg) extraConfig extensions search;
          inherit (config.${namespace}.user) name;

colonelpanic8 pushed a commit to colonelpanic8/home-manager that referenced this pull request Dec 30, 2024
Split off from nix-community#5697, organizes firefox tests by submodule.

This is intended to match directory structure setup for the new search submodule.
colonelpanic8 pushed a commit to colonelpanic8/home-manager that referenced this pull request Dec 30, 2024
* firefox: split search into separate submodule file

* thunderbird: support setting search engines
@kira-bruneau kira-bruneau deleted the thunderbird-search-engines branch February 4, 2025 21:00
khaneliman pushed a commit that referenced this pull request Mar 19, 2025
This splits the bookmarks submodule into a seperate file, to make it easier to maintain (like how the search module was previously split out in #5697).

This also refactors bookmarks to require a new force option, to be more explicit about overriding existing bookmarks.
nfelber pushed a commit to nfelber/home-manager that referenced this pull request Mar 19, 2025
…munity#6402)

This splits the bookmarks submodule into a seperate file, to make it easier to maintain (like how the search module was previously split out in nix-community#5697).

This also refactors bookmarks to require a new force option, to be more explicit about overriding existing bookmarks.
pasqui23 pushed a commit to pasqui23/home-manager that referenced this pull request Apr 25, 2025
…munity#6402)

This splits the bookmarks submodule into a seperate file, to make it easier to maintain (like how the search module was previously split out in nix-community#5697).

This also refactors bookmarks to require a new force option, to be more explicit about overriding existing bookmarks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mail HM email accounts, thunderbird, alot, notmuch, msmtp, meli...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants