fix: type error in case config.bindings = false.

[troiganto]

The docs say this:

To disable all bindings, set the bindings field to false

but the keybindings code accessed config.bindings.prefix unconditionally, causing a type error if config.bindings is the boolean value false. This PR fixes this by fetching prefix from the local bindings variable, which defaults to {} if a false value was configured.

We perform the same fix for the dailies extension. It's a bit unclear what happens in this case:

require("org-roam").setup({
  -- ...
  bindings = false, -- `bindings.prefix` no longer exists.
  extensions = {
    dailies = {
      bindings = {
        goto_tomorrow = "<prefix>ndt", -- what happens here?
      },
    },
  },
})

My understanding of the code is that <prefix> will remain unexpanded in this case.

[chipsenkbeil]

Hm, maybe this got broken as a result of #47.

The expectation was that setting bindings to nil will disable setting any bindings. If you want to turn off the roam bindings, you do it at the top level. If you want to turn off the extension bindings, you do it at the extension.

So disabling them both would be required, but I wrote the documentation before the dailies extension was created.

[troiganto]

I'm willing to extend this fix so that it behaves correctly, but from your message, I'm not sure which behavior you'd prefer to see.

Should assign_dailies_keybindings raise an error if it detects that roam.config.bindings is nil but roam.config.extensions.dailies.bindings isn't? I think that'd be the least surprising and also reasonably simple to add.

[chipsenkbeil]

Should assign_dailies_keybindings raise an error if it detects that roam.config.bindings is nil but roam.config.extensions.dailies.bindings isn't? I think that'd be the least surprising and also reasonably simple to add.

I think that's totally fair to support. I'm up for anything that seems intuitive and reasonable. 😄

Did you want to make that addition in this PR? If so, we can do that and update the documentation.

[troiganto]

Alright, I've thought about it some more and ended up just silently disabling dailies bindings if core bindings are disabled. Otherwise, someone who wanted no default bindings whatsoever would've had to override both, which seemed overly annoying.

I've also added tests for this behavior and adjusted the docs. Feel free to let me know if you'd like anything changed, and also feel free to make any changes yourself! 🙂

[chipsenkbeil]

Looks good to me! Thanks for the contribution.