executablebooks / mdformat

CommonMark compliant Markdown formatter
https://mdformat.rtfd.io
MIT License
437 stars 46 forks source link

Support XDG base directory specification for user configuration resolution #400

Open rpdelaney opened 1 year ago

rpdelaney commented 1 year ago

Context

In #263 we added support for .mdformat.toml as an on-disk configuration file.

I would like to revisit the possibility of also reading fall-back configuration from $XDG_CONFIG_HOME.

platformdirs provides tools that can assist with this in a cross-platform way.

Proposal

The best approach I could think of is to start with the lowest priority configuration files first and work down, applying changes as we go. That way, defaults can be over-ridden by $XDG_CONFIG_HOME/mdformat.toml (or whatever we call it) but project-specific settings still take priority over any other on-disk configuration.

Descending priority would look something like (subject to negotiation):

  1. $HOME/.mdformat.toml
  2. $HOME/.config/mdformat.toml
  3. $XDG_CONFIG_HOME/mdformat.toml
  4. /.mdformat.toml
  5. /path/to/repo/.mdformat.toml

Tasks and updates

RemasteredArch commented 3 months ago

Hey there, I'm thinking about doing this. I'm apprehensive because I don't have any experience with large codebases (which notably means I've never written a test) or Python. I would be willing to learn those to do this, but I'm not sure if this would end up being more work than think it will be. Would this be a suitable first contribution?

From what I understand from a little bit of reading the code, the changes involved would look something like this:

src/mdformat/_cli.py:

# ...

- from mdformat._conf import DEFAULT_OPTS, InvalidConfError, read_toml_opts
+ from mdformat._conf import DEFAULT_OPTS, InvalidConfError, read_toml_opts, read_user_opts

# ...

 for path in file_paths:
         try:
+            # Looks for a config in user directories, e.x. $XDG_CONFIG_HOME
+            user_opts = read_user_opts()
+            # Looks for a config in parent directories
-            toml_opts = read_toml_opts(path.parent if path else Path.cwd())
+            proj_opts = read_toml_opts(path.parent if path else Path.cwd())
         except InvalidConfError as e:
             print_error(str(e))
             return 1
-        opts: Mapping = {**DEFAULT_OPTS, **toml_opts, **cli_opts}
+        opts: Mapping = {**DEFAULT_OPTS, **user_opts, **proj_opts, **cli_opts}

         if path:
             path_str = str(path)
             original_str = sys.stdin.read()

         formatted_str = mdformat.text(
             original_str,
             options=opts,

# ...

src/mdformat/_conf.py: \<implement reading config directory checks using platformdirs, probably about 30 lines>

tests/test_config_file.py: \<implement tests for the above, probably about 30 lines>

docs/users/configuration_file.md:

 Mdformat allows configuration in a [TOML](https://toml.io) file named `.mdformat.toml`.

-The configuration file will be resolved starting from the location of the file being formatted,
-and searching up the file tree until a config file is (or isn't) found.
-When formatting standard input stream, resolution will be started from current working directory.
+The configuration file will be resolved from two sources:

-Command line interface arguments take precedence over the configuration file.
+1. The user's configuration directory.
+   <list OS-specific location directories, e.g. `$XDG_CONFIG_HOME/mdformat` or `~/.config/mdformat` on Linux>
+1. Starting from the location of the file being formatted,
+   and searching up the file tree until a config file is (or isn't) found.
+   When formatting standard input stream, resolution will be started from current working directory.
+
+The configuration file from the file tree take precedence over the user's configuration directory.
+Command line interface arguments take precedence over the configuration files.

 ## Example configuration

 <!-- ... -->

I'm not certain if my estimates of this being not terribly difficult are incorrect; and I don't want to be a burden on contributors by trying to merge bad code.

rpdelaney commented 3 months ago

Skimming over it, your analysis seems reasonable. @hukkin Would you consider a PR that implements the feature in the OP?

hukkin commented 1 week ago

Hey, thanks for the issue!

I think I'll reject a global config for the same reasons that Prettier does so. Quoting Prettier docs

Prettier intentionally doesn’t support any kind of global configuration. This is to make sure that when a project is copied to another computer, Prettier’s behavior stays the same. Otherwise, Prettier wouldn’t be able to guarantee that everybody in a team gets the same consistent results.

Also with how the new exclude configuration works, I don't think a global config can be intuitive. What would the project root for file exclusions be? Has to be current working directory I think for it to make any sense, but then behavior starts to get increasingly unexpected when running mdformat from child directories of the "user expected" project root.

rpdelaney commented 1 week ago

Prettier does so.

Prettier feels like the wrong baseline since so many python tools that do things like mdformat allow this. See this comment for more of my rationale on this.

Also with how the new exclude configuration works, I don't think a global config can be intuitive.

I disagree. For example, ruff supports sophisticated options for selecting and excluding checks in various scenarios. The solution is to implement whatever configuration is found first in the discovery hierarchy. If there is no project configuration then you can fall back to the global configuration safely (since if consistency were required, there would be one).

hukkin commented 5 days ago

Prettier feels like the wrong baseline since so many python tools that do things like mdformat allow this.

Python is an implementation detail for mdformat. The CLI is not a Python tool, it's a Commonmark formatter. Commonmark has nothing to do with Python. The API admittedly is a "Python tool" but that does not interact with configuration files.

I disagree.

If you disagree, maybe you can explain how the project root for the purposes of exclude would be resolved with an XDG base directory.


If you really want, you can already create a /home/rpdelaney/.mdformat.toml or even /.mdformat.toml which effectively is a global configuration, but doesn't have the problem with exclude and deciding project root dir.