jscheid / prettier.el

Prettier code formatting for Emacs.
GNU General Public License v3.0
163 stars 11 forks source link

automatically enable prettier-mode for projects with any prettier config #130

Open aspiers opened 6 months ago

aspiers commented 6 months ago

Context

If I understand correctly, out of the box prettier.el basically offers two ways for it to be automatically activated when visiting a file:

  1. enabling global-prettier-mode will automatically activate for all enabled parsers
  2. via per-directory local variables

Whilst neither of these are bad options, they are not perfect:

  1. global-prettier-mode has the undesirable effect of often enabling prettier when I don't want it to, e.g. on projects which do not use prettier.
  2. When not using global-prettier-mode, setting up per-directory local variables is significant extra work for the user:
    • It needs to be done per-project.
    • It may need to be done per-mode.
    • It requires remembering the somewhat complex syntax for per-directory local variables.
    • It breaks if you decide to switch major modes, e.g. from typescript-mode to the more modern typescript-ts-mode based on tree-sitter).
    • It litters directories with .dir-locals.el.

Feature request

Consequently, I would love to see prettier.el support an additional approach based on the DWIM philosophy, where the mode is automatically activated for every file in a project which has prettier configuration.

In other words, if a project (git repository or similar) contains any files like .prettierrc, .prettierrc.* which are valid configuration files), or even .prettierignore, I want prettier.el to automatically activate for any files in that project.

Stretch goal

In fact, ideally it would only activate for the files in that project which are governed by prettier, and not for files which are ignored by a .prettierignore file in the project's root, but that's an extra nice-to-have feature which could come later.

Backwards-compatibility

Since this would only offer a new mechanism for automatic activation, it would not interfere with any existing local variables, or even with global-prettier-mode. So it should be completely backwards-compatible.

Rationale

The rationale for this feature request is pretty simple:

  1. If a project has prettier config, that config was put there for a reason, so by default it should obviously be respected.
  2. It avoids placing any unnecessary burden on the user. The user just gets what they want automagically. DWIM for the win!
jscheid commented 6 months ago

Hi Adam, sounds like a great suggestion, thank you. I will say that it solves a problem that doesn't bother me personally. I can't say exactly why but there's never a moment where I think "I wish prettier-mode wasn't enabled in this buffer!" But maybe that has to do with my workflows vs. yours.

If you're interested in working on this I won't be in the way, as long as it's opt-in. There are a few possible complications that you may have considered already, but let me mention them here just in case you haven't:

  1. Some people configure Prettier through a section in package.json rather than through .prettierrc. (I work on some projects that do.)
  2. Some like to use Prettier with just the defaults, without any configuration file at all.
  3. Some have a .prettierrc higher up in the hierarchy, maybe in their home directory. Pretty sure one person opened an issue about that once in this repo.
  4. In a monorepo, you might have a .prettierrc file at the root but might not necessarily want to use Prettier in all packages, i.e. subdirectories.

FYI I have a mid- to long-term plan to replace the functionality provided by this package by a completely new approach based around LSP. There are a few upsides to this, such as consistent support for non-Prettier formatters such as rustfmt or the recently announced Biome, as well as a less complicated MELPA setup and synergies around the transport layer. It would also be even lower latency, thanks to the server keeping a copy of all documents in memory. It's not a fully formed idea yet but I have started to investigate some basic building blocks.

Anyway, the reason I mention this is because I was planning to make configuration more deliberate, not less, by means of some kind of config file that you put at the root of your project (and/or workspace). It wouldn't be as "automagic" as you envision but as per the Zen of Python, explicit is better than implicit and besides, putting a config file in your root once and then almost never touching it again isn't too much to ask for IMHO. It's not DWIM but it also is, in a way, if you consider that you put "meaning" into the file.

jscheid commented 6 months ago

One more thought: disregarding for a moment that the solution you're pursuing might be useful to others: what exactly is keeping you from building a (hopefully short) piece of Lisp code that would do what you want outside of the package? Something based around hooks, maybe with a helper function or two. It's not a rhetorical question, there may be something not exposed by this package that prevents you from doing so. If there isn't, perhaps that would be a good first step, just because it would let you (and others) test how well it works in practice? I'm just thinking out loud, it's entirely possible that a change to this repo would be the best way to implement it.

aspiers commented 6 months ago

Hi Adam, sounds like a great suggestion, thank you. I will say that it solves a problem that doesn't bother me personally. I can't say exactly why but there's never a moment where I think "I wish prettier-mode wasn't enabled in this buffer!" But maybe that has to do with my workflows vs. yours.

Thanks a lot for the quick response! Interesting that this isn't a problem for you. In case you're curious, here's the context behind why it's an issue for me:

I regularly collaborate on a wide mix of repositories (both professionally and as a F/OSS hobbyist). Some -- but far from all -- of these use Prettier. Obviously on the ones which do, I want to use prettier-mode. But if I enabled prettier-mode on the projects which don't use it, it would reformat huge amounts of code in ways that the maintainer(s) don't want and never asked for. This would typically lead to PRs with tons of unwanted reformatting, which would typically be rejected, possibly even annoying people in the process. So obviously this is a huge no-no! Not just for me, but for anyone who regularly contributes to a mix of projects which do and don't use prettier.

If you're interested in working on this I won't be in the way, as long as it's opt-in. There are a few possible complications that you may have considered already, but let me mention them here just in case you haven't:

  1. Some people configure Prettier through a section in package.json rather than through .prettierrc. (I work on some projects that do.)

Great point. That could be something to worry about later, after the basic functionality is done.

  1. Some like to use Prettier with just the defaults, without any configuration file at all.

Sure. In those cases, this feature would yield an additional option on top of the two you already provide: just creating an empty .prettierrc, which is probably good practice anyway since it would signal to editors other than emacs that the repository expects to be formatted via prettier.

  1. Some have a .prettierrc higher up in the hierarchy, maybe in their home directory. Pretty sure one person opened an issue about that once in this repo.
  2. In a monorepo, you might have a .prettierrc file at the root but might not necessarily want to use Prettier in all packages, i.e. subdirectories.

Yeah, these need some thought. Perhaps projectile-locate-dominating-file or the built-in locate-dominating-file or something similar could help with these, which begs the question of whether to achieve this via optional integration with projectile.el and/or project.el.

FYI I have a mid- to long-term plan to replace the functionality provided by this package by a completely new approach based around LSP. There are a few upsides to this, such as consistent support for non-Prettier formatters such as rustfmt or the recently announced Biome, as well as a less complicated MELPA setup and synergies around the transport layer. It would also be even lower latency, thanks to the server keeping a copy of all documents in memory. It's not a fully formed idea yet but I have started to investigate some basic building blocks.

Sounds interesting! Is there already an issue tracking that? If so, I'd love to subscribe to monitor progress.

Anyway, the reason I mention this is because I was planning to make configuration more deliberate, not less, by means of some kind of config file that you put at the root of your project (and/or workspace).

Well, isn't that essentially what I'm proposing here? Prettier already honours configuration files, so prettier.el could just piggy-back on top of those rather than requiring its own config file which imposes more burden on users.

It wouldn't be as "automagic" as you envision but as per the Zen of Python, explicit is better than implicit

I'm not convinced that the context in which that principle was expounded applies here, as presumably it was intended to apply more to writing Python code than to configuring tools. My personal belief, which I think is very widely held, is that when there is a good DWIM option available, that is what the majority of users want as the default behaviour. Otherwise we'd all have to spend far too many hours reinventing countless configuration wheels. That's why emacs starter kits like Spacemacs and Doom are so popular, and also why VS Code is so wildly successful -- they are packed with sensible, useful stuff out of the box with little or no extra config required.

and besides, putting a config file in your root once and then almost never touching it again isn't too much to ask for IMHO. It's not DWIM but it also is, in a way, if you consider that you put "meaning" into the file.

That's fine, but I don't see how it solves the problem I described above, where prettier.el gets undesirably activated for some projects which don't use Prettier.

jscheid commented 6 months ago

This would typically lead to PRs with tons of unwanted reformatting, which would typically be rejected, possibly even annoying people in the process. So obviously this is a huge no-no! Not just for me, but for anyone who regularly contributes to a mix of projects which do and don't use prettier.

Maybe the difference is that you have Prettier installed globally and I don't? Anyway, it seems to be a problem for you so definitely happy to explore your solution.

Sounds interesting! Is there already an issue tracking that? If so, I'd love to subscribe to monitor progress.

Glad to hear. It won't be an issue in this repo, it will be a separate project. I'll make sure to give you a heads up if and when it comes to something.

packed with sensible, useful stuff out of the box with little or no extra config required.

Like everyone, I also don't enjoy needless ceremony. However, I've noticed over the years that I also like to know what's going on, just so that I know how to disable it if I don't like it, or debug it if it's malfunctioning. Call me a control freak but that's generally been working out well for me.

Yes, editors are different from programming languages and the Zen of Python was written in a different context, so it was probably unwise to quote it, but the same basic principle still holds for me when it comes to Emacs. For instance I dislike that all the existing LSP clients "automatigally" configure all these helpful changes, most of which I don't enjoy very much in the form they exist by default. The LSP setup that I envision is much more opt-in than that, on a more granular level.

I acknowledge that this is oversimplifying things and also that there is a sliding scale here, and that there are different people with different preferences. You don't have to convince me of anything, and I'm not trying to convince you, just throwing in my 2c.

That's fine, but I don't see how it solves the problem I described above, where prettier.el gets undesirably activated for some projects which don't use Prettier.

I just meant that (the future package as I envision it) will only activate in the presence of the file. A bit like what you were saying with an empty .prettierrc file, except that it would be a separate file because the tool would eventually be about more than just formatting with Prettier. But let's not discuss this too much here since I was talking about hypothetical functionality that isn't really too much related to your plans.

Just to reiterate: go for it, and let me know how I can help. The parts of the code that this is touching should be pretty straightforward I hope, or maybe you don't need to touch the code at all (see my other comment.)

aspiers commented 6 months ago

One more thought: disregarding for a moment that the solution you're pursuing might be useful to others: what exactly is keeping you from building a (hopefully short) piece of Lisp code that would do what you want outside of the package?

Nothing stops that technically. But I thought it made sense to first check with you a) that there isn't already a mechanism for this which I missed, and b) what's your take on this feature and whether you'd be open to accepting PRs for it. Well, to be perfectly honest, I was hoping that you would want the feature for yourself and end up implementing it quicker than I could 😆😜

Something based around hooks, maybe with a helper function or two.

Yes, it sounds quite likely that this is entirely possible.

It's not a rhetorical question, there may be something not exposed by this package that prevents you from doing so. If there isn't, perhaps that would be a good first step, just because it would let you (and others) test how well it works in practice? I'm just thinking out loud, it's entirely possible that a change to this repo would be the best way to implement it.

Absolutely! I guess it could just use find-file-hook to execute a function which looks for prettier config. There's some good news here: Prettier offers a --find-config-path option, which is basically exactly what we need. I just tested it in a local repo:

$ pnpm exec prettier --find-config-path src/shared/parser.ts; echo $?
.prettierrc
0
$ rm .prettierrc
$ pnpm exec prettier --find-config-path src/shared/parser.ts; echo $?
[error] Can not find configure file for "src/shared/parser.ts".
1

But then we are back to the original problem which prettier.el solves so nicely: we definitely don't want the extra latency of firing up a new Prettier process every time we open a file in emacs.

So I guess the first question for you would be whether it's possible to invoke --find-config-path via prettier.el, or if not, whether support for that could be added fairly easily?

jscheid commented 6 months ago

So I guess the first question for you would be whether it's possible to invoke --find-config-path via prettier.el, or if not, whether support for that could be added fairly easily?

~We're already using that, the API equivalent is resolveConfig which you can grep for in our code. It might be only a matter of aEcting on its result differently on the Emacs side.~

EDIT: actually no, the equivalent is resolveConfigFile which we're not using yet, but could without too much work.

jscheid commented 6 months ago

@aspiers so here's on thing that might work: when prettier first loads in a buffer, ask the server to check whether there's a config file, and disable itself if there isn't. That might be a good improvement indeed.

aspiers commented 6 months ago

This would typically lead to PRs with tons of unwanted reformatting, which would typically be rejected, possibly even annoying people in the process. So obviously this is a huge no-no! Not just for me, but for anyone who regularly contributes to a mix of projects which do and don't use prettier.

Maybe the difference is that you have Prettier installed globally and I don't?

Ahh, maybe I misunderstood you before, because I'm a bit confused now. You previously wrote:

there's never a moment where I think "I wish prettier-mode wasn't enabled in this buffer!"

I took this to mean that you use global-prettier-mode, because if instead you were manually enabling it per directory/mode then the above statement would have seemed rather tautological and I'm not sure what point it would have been making. So how do you activate the mode? Globally, or manually, or is there a third option I'm not yet aware of?

If you're doing it manually, then I guess your point would be that it never bothered you having to do that extra configuration work?

Anyway, it seems to be a problem for you so definitely happy to explore your solution.

Thanks, I really appreciate your time on this :)

Sounds interesting! Is there already an issue tracking that? If so, I'd love to subscribe to monitor progress.

Glad to hear. It won't be an issue in this repo, it will be a separate project. I'll make sure to give you a heads up if and when it comes to something.

Cool, sounds great!

packed with sensible, useful stuff out of the box with little or no extra config required.

Like everyone, I also don't enjoy needless ceremony. However, I've noticed over the years that I also like to know what's going on, just so that I know how to disable it if I don't like it, or debug it if it's malfunctioning. Call me a control freak but that's generally been working out well for me.

Oh yes, I'm 100% with you on the need to understand things and debug / disable them if needed! That doesn't negate the value of a well thought-out DWIM setup though :)

Yes, editors are different from programming languages and the Zen of Python was written in a different context, so it was probably unwise to quote it, but the same basic principle still holds for me when it comes to Emacs. For instance I dislike that all the existing LSP clients "automatigally" configure all these helpful changes, most of which I don't enjoy very much in the form they exist by default. The LSP setup that I envision is much more opt-in than that, on a more granular level.

I acknowledge that this is oversimplifying things and also that there is a sliding scale here, and that there are different people with different preferences. You don't have to convince me of anything, and I'm not trying to convince you, just throwing in my 2c.

Sure - I can certainly see the argument from both sides. One idiomatic solution in emacs would be to ensure that all the configuration can be managed by custom variables, and then offer at least two themes out of the box, e.g. one minimalist config theme, and one with all the bells and whistles enabled. This would be an easy way to satisfy both crowds.

That's fine, but I don't see how it solves the problem I described above, where prettier.el gets undesirably activated for some projects which don't use Prettier.

I just meant that (the future package as I envision it) will only activate in the presence of the file. A bit like what you were saying with an empty .prettierrc file, except that it would be a separate file because the tool would eventually be about more than just formatting with Prettier. But let's not discuss this too much here since I was talking about hypothetical functionality that isn't really too much related to your plans.

Oh I see! So by "putting a config file in your root" I guess you meant the root of the project - for some stupid reason I had assumed you meant the home directory, for enabling it globally.

Just to reiterate: go for it, and let me know how I can help. The parts of the code that this is touching should be pretty straightforward I hope, or maybe you don't need to touch the code at all (see my other comment.)

👍🏼

So I guess the first question for you would be whether it's possible to invoke --find-config-path via prettier.el, or if not, whether support for that could be added fairly easily?

~We're already using that, the API equivalent is resolveConfig which you can grep for in our code. It might be only a matter of aEcting on its result differently on the Emacs side.~

EDIT: actually no, the equivalent is resolveConfigFile which we're not using yet, but could without too much work.

Is that something you might be able to do easily as an enabling step? I'm afraid I currently have almost zero knowledge of prettier.el's internals.

@aspiers so here's on thing that might work: when prettier first loads in a buffer, ask the server to check whether there's a config file, and disable itself if there isn't. That might be a good improvement indeed.

Yes indeed - not just a good improvement, but it would entirely solve my use case, with nothing else needed! It could simply be an extension of global-prettier-mode's existing behaviour, controlled via a custom variable called global-prettier-mode-require-config or something like that.