Skip to content

Conversation

@fk29g
Copy link
Contributor

@fk29g fk29g commented Oct 12, 2025

Description

Adds a module for configuring SQLite.

Checklist

  • Change is backwards compatible.

  • Code formatted with nix fmt or
    nix-shell -p treefmt nixfmt deadnix keep-sorted --run treefmt.

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

  • 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.
    • Generate a news entry. See News
    • Basic tests added. See Tests

@khaneliman
Copy link
Collaborator

Can you squash/fixup the last 2 commits into their relevant commit

@fk29g fk29g force-pushed the add-module-sqlite branch from b57e7eb to 46835a4 Compare October 13, 2025 12:45
Comment on lines +108 to +134
xdg.configFile."sqlite3/sqliterc".text = lib.concatStringsSep "\n" (
lib.filter (x: x != "") [
(lib.optionalString (cfg.mode != null) ".mode ${cfg.mode}")
(lib.optionalString (
cfg.separator.column != null && cfg.separator.row != null
) ".separator \"${cfg.separator.column}\" \"${cfg.separator.row}\"")
(lib.optionalString (
cfg.separator.column != null && cfg.separator.row == null
) ".separator \"${cfg.separator.column}\"")
(lib.optionalString (
cfg.separator.column == null && cfg.separator.row != null
) ".separator \"${defaultColSeparator}\" \"${cfg.separator.row}\"")
(lib.optionalString (
cfg.prompt.main != null && cfg.prompt.continue != null
) ".prompt \"${cfg.prompt.main}\" \"${cfg.prompt.continue}\"")
(lib.optionalString (
cfg.prompt.main != null && cfg.prompt.continue == null
) ".prompt \"${cfg.prompt.main}\"")
(lib.optionalString (
cfg.prompt.main == null && cfg.prompt.continue != null
) ".prompt \"${defaultPrompt}\" \"${cfg.prompt.continue}\"")
(lib.optionalString (cfg.extraConfig != "") (cfg.extraConfig))
]
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I worry about this being too restrictive and also dismissive of configuration. This contains a lot of conditional logic when it seems like the file is a basic syntax of .<option> key value or something? Maybe we can make/use a generator that allows more freeform configuration?

If not interested in a generator, I think it'd make more sense to create warnings/assertions when combining mutually exclusive options instead of just silently choosing which to apply.

Copy link
Member

Choose a reason for hiding this comment

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

In any case, it should probably have an escape hatch of some sort. This is not a fancy looking declarative configuration, it's just a script that gets run upon running sqlite. It can include arbitrary SQL in addition to the . commands. It's pretty common to use it to enable foreign key constraints for example.

@fk29g
Copy link
Contributor Author

fk29g commented Oct 14, 2025 via email

options.programs.sqlite = {
enable = lib.mkEnableOption "sqlite";

package = lib.mkPackageOption pkgs "sqlite" { nullable = true; };
Copy link
Member

Choose a reason for hiding this comment

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

Assuming this is intended for interactive use?

Suggested change
package = lib.mkPackageOption pkgs "sqlite" { nullable = true; };
package = lib.mkPackageOption pkgs "sqlite-interactive" {
nullable = true;
example = [ "sqlite" ];
};

fk29g added 3 commits October 18, 2025 12:23
Add tests for an empty and decently realistic example configuration.
@fk29g
Copy link
Contributor Author

fk29g commented Oct 18, 2025 via email

@fk29g fk29g force-pushed the add-module-sqlite branch from 46835a4 to 3878dd8 Compare October 18, 2025 10:30
@fk29g
Copy link
Contributor Author

fk29g commented Oct 18, 2025 via email

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