On Thu, Nov 21 2019, Johan Parin wrote:
> Johan Parin <johanparin@gmail.com> writes:
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>>
>>> Is it that much worse to pass around the notmuch_database_t *?

My opinion is that we should not use the static notmuch_database_t
pointer. I think any desire to reach for that is, at best, papering over
some other problem that would be better fixed itself.

>> It has a lot of code impact, it really propagates to a lot of
>> places.
...
> Here is a taste (not fully tested, but seems to work).
...
>  static int
> -do_reply (notmuch_config_t *config,
> +do_reply (notmuch_database_t *notmuch,
> +	  notmuch_config_t *config,

This passing around of the pair of notmuch_database_t* and a
notmuch_config_t* all over the place looks like an anti-pattern to me,
(and something that was just being hidden by the static
notmuch_database_t* that would be better to be fixed properly).

One option for reducing that pair to a single pointer would be to move
the configuration into the database, (as Daniel has been arguing for a
while).

I've argued against that in the past, (and obviously, I came up with the
current approach originally). But the historical situation has already
led to some unpleasant mixing of configuration state, (where some state
is in the configuration file and other state is in the database and it's
hard to predict which is where and which things can be controlled in the
configuration file).

And I think a patch like the above with the "notmuch, config" all over
the place would be another unpleasant result of the historical
convention.

So, I wouldn't be opposed to having the configuration move into the
database entirely. I would definitely like to see a tool that allows for
a dump/restore operation of the configuration state, (so users can still
edit configuration parameters with a text editor and with
fully-documented parameters). Bonus points if the users can also capture
their own commentary/explanation of values they are setting (as they can
do with the current configuration file).

And if things do move, existing configuration files should be updated
with a comment describing what that new dump/restore mechanism is and
that the current configuration file will no longer be used, (though, at
the transition point, obviously configuration parameters should be
copied from the configuration file into the database).

-Carl