zyedidia / micro

A modern and intuitive terminal-based text editor
https://micro-editor.github.io
MIT License
24.42k stars 1.16k forks source link

plugin: formatter: Add plugin to format files #3166

Open taconi opened 4 months ago

taconi commented 4 months ago

Today the computer does not have an official way of creating formatters, that is, if I want to do this I will have to create a plugin for it. Plugins are created for a single formatter, sometimes by more than one person.

The idea of ​​this pull request is to add a built-in plugin for you to create formatters of files for all types of languages.

Here is an example:

-- ~/.config/micro/init.lua

formatters = {
  {
    cmd = 'goimports -w %f',
    onSave = true,
    filetypes = { 'go' },
  },
  {
    cmd = 'clang-format -i %f',
    filetypes = { 'c', 'c++', 'csharp' },
    bind = 'Alt-l'
  },
  {
    cmd = 'mix format %f',
    filetypes = { '\\.(exs?)' },
    domatch = true,
  },
  {
    cmd = 'stylua %f',
    args = {
      '--column-width=120',
      '--quote-style=ForceSingle',
      '--line-endings=Unix',
      '--indent-type=Spaces',
      '--call-parentheses=Always',
      '--indent-width=2',
    },
    filetypes = { 'lua' },
  },
}

function init()
  formatter.setup(formatters)
end

For more details see help.

JoeKar commented 3 months ago

One thought about the duplication of documentation, not related to this new plugin in the first place, but in general to all default plugins: https://github.com/zyedidia/micro/blob/a57d29ada9db191a46a569d5203dc600ab2f76ba/runtime/help/options.md?plain=1#L471-L481 https://github.com/zyedidia/micro/blob/a57d29ada9db191a46a569d5203dc600ab2f76ba/runtime/help/plugins.md?plain=1#L416-L426

So the reduced description is duplicated and needs to be maintained twice. Maybe we can link from that default plugin section in options.md to plugins.md#default-plugins. The text above the default plugin list in options.md shall be kept to describe that every plugin will get his own config switch.

What do you think about this?

taconi commented 3 months ago

I really don't know if there is a need to have plugin descriptions in the options help, maybe we can link the plugin help and we can use the true option, which is the value of the option.

Plugin options: all plugins come with a special option to enable or disable
them. The option is a boolean with the same name as the plugin itself.

By default, the following plugins are provided, each with an option to activate
or disable them (all are `true` by default):

* `autoclose`
* `comment`
* `diff`
* `formatter`
* `ftoptions`
* `linter`
* `literate`
* `status`

> For more details about these plugins, see plugins help (`> help plugins`)

Would that solve it? There would still be options to deactivate the plugins but the description would only be in the help of the plugins

JoeKar commented 3 months ago

Would that solve it?

Not really, since we still have to keep track of the plugin list twice. The available plugins (including their description) are listed in the plugin help itself, so we don't need to list them once again.

Maybe:

Plugin options: all plugins come with a special option to enable or disable them. The option is a boolean with the same name as the plugin itself. By default, the value of this option is true.

For more details about these plugins, see plugins help (plugins.md#default-plugins. or > help plugins)

taconi commented 3 months ago

Plugin options: all plugins come with a special option to enable or disable them. The option is a boolean with the same name as the plugin itself. By default, the value of this option is true.

For more details about these plugins, see plugins help (plugins.md#default-plugins. or > help plugins)

Okay for me

JoeKar commented 3 months ago

One thing which may be discussed is the situation "default" plugin or not (added to the plugin-channel). I've no problem with one or the other. Most probably depends on the decision if the plugin is "fundamental" enough to be deployed with micro itself. A formatter subsystem could be one of those.

@dmaluka Do you share that opinion or do disagree?

taconi commented 3 months ago

One thing which may be discussed is the situation "default" plugin or not (added to the plugin-channel).

If you are talking about the manager plugin, it does more things besides managing formatters, so the formatter is not a plugin that will be added from the plugin-channel (if it is the same it still lives), but as soon as the formatter plugin is accepted and released a version with it the manager will use its interface just as it uses the linter plugin to manage linters.

Most probably depends on the decision if the plugin is "fundamental" enough to be deployed with micro itself. A formatter subsystem could be one of those.

I agree. A way for the user to be able to create formatters should come by default, just like the linter plugin.

dmaluka commented 3 months ago

One thing which may be discussed is the situation "default" plugin or not (added to the plugin-channel). I've no problem with one or the other. Most probably depends on the decision if the plugin is "fundamental" enough to be deployed with micro itself. A formatter subsystem could be one of those.

@dmaluka Do you share that opinion or do disagree?

Ok, I've found a moment to look at this. So I've been trying to understand what is this all about, what is a "formatter" and what the heck is this plugin doing. I've looked at the documentation, which is supposed to explain these things in its initial section, but instead the documentation begins with some mumbo-jumbo:

# Formatter

Formatter settings can be for any type of file (javascript, go, python), but you
need to have the cli's you want to use as formatter installed.
For example, if you want to configure a formatter for python files and decide to
use [blue], you would need to have it installed on your machine.

Here is an example with [blue]:

Sorry, I'm not sure what am I supposed to do with this.

So, I don't know if this plugin is "fundamental" enough and what should I agree or disagree with, since I don't really understand what is this plugin all about.

taconi commented 3 months ago
# Formatter

With this plugin you can configure cli's that will be used to format your codes.

Does this text meet the need to explain what the plugin does? @dmaluka

taconi commented 3 months ago

The idea would be to eliminate the need for the user to write their own plugin to format files. Currently that I know of, there are the following plugins to solve this (some of them do other things besides formatting files, like crystal, go and manager):

But how would someone format an elixir file? Creating a plugin to execute the mix format file.ex command A rust file? Creating another plugin to execute the command rustfmt +nightly file.rs A racket file? Another plugin to execute a command in the terminal.

We would be providing a way for the user to define a file formatter without having to create a new plugin for it.

Just open ~/.config/micro/init.lua and list your formatters:

function init()
  formatter.setup({
    { cmd = 'mix format %f', bind = 'Alt-m', filetypes = { 'elixir' } },
    { cmd = 'rustfmt +nightly %f', onSave = true, filetypes = { 'rust' } },
    { cmd = 'raco fmt -i %f', domatch = true, filetypes = { '\\.rkt$' } }, 
  })
end
dmaluka commented 3 months ago

Ok, got it. Yeah, it seems reasonable to have such a unified formatter plugin as a default plugin, like the linter plugin we already have.

But regarding the way how you implemented it, I have a fundamental question: why not implement the formatter plugin "just like the linter plugin", to keep things simple and consistent? I.e. just formatter.makeFormatter() per each formatter, with all the needed params, similar to the linter.makeLinter(). Why do you need to invent your own configuration system instead?

This question also concerns the documentation. Look at help linter, that is how a clear and concise documentation looks like. Which I cannot say about your documentation. So why not just follow linter as an example?

taconi commented 3 months ago

why not implement the formatter plugin "just like the linter plugin", to keep things simple and consistent?

The linter plugin API is not cool at all, linter.makeLinter receives 11 parameters, if I want to pass the mandatory parameters and the last parameter I need to pass all the other optional parameters:

function init()
  linter.makeLinter(
    'rubocop',                      -- name         : required
    'ruby',                         -- filetype     : required
    'rubocop',                      -- cmd          : required
    { '--format', 'emacs', '%f' },  -- args         : required
    '%f:%l:%c: C: %m',              -- errorformat  : required
    nil,                            -- os
    false,                          -- whitelist
    false,                          -- domatch
    0,                              -- loffset
    0,                              -- coffset
    function(buf)                   -- callback
      return false
    end
  )
end

I need to pass os, whitelist, domatch, loffset and coffset to be able to pass my callback.

The API was implemented based on the formatters found in lunarvim. Using a lua table you can define only the keys you want and those that are not defined the plugin will set their default value.

But I can implement a formatter.makeFormatter for those who think the linter plugin interface is "simple and consistent".

Look at help linter, that is how a clear and concise documentation looks like. Which I cannot say about your documentation

Regarding documentation, I don't know if the linter documentation is good because it doesn't give examples of using the features it has and peculiar behaviors such as the behavior of domatch trying to match the name of the file type (such as python, ruby, unknown), which doesn't make any sense (related to #3156), goes unnoticed.

The idea of​the formatter plugin documentation was to document the plugin, teach the user with examples so that they can actually make use of the features and even copy the example for use, perhaps it is not written in the best way but put a doc like the linter it would be asking the user to find out how to use the plugin or asking how to use it (#3164).

I would let users use micro's built-in keybinding system instead of introducing a plugin-specific "bind" option for each formatter.

But what would be the problem with making the user write bind = 'Alt-m' so that he has to write local config = import('micro/config') and config.TryBindKey('Alt-m' , 'command:goimp', true) ? Since we can simplify the use, why would I continue with the complex/verbose?

How about using the same format command for all formatters? So you would use format goimp instead of goimp. That would let you get rid of the entire makeCommands option which introduces a lot of complexity and needs a lengthy explanation in the documentation.

The format command already exists and it runs all formatters it finds for the current file, but passing the name of the formatter to this command as a filter? 🤯 Genius, I'm going to implement this.

taconi commented 3 months ago

@dmaluka How could we improve the formatter plugin documentation without having to remove the examples and explanations for each field? (the makeCommands option will be removed so its documentation too)

What points need to be improved?

dmaluka commented 3 months ago

@dmaluka How could we improve the formatter plugin documentation without having to remove the examples and explanations for each field? (the makeCommands option will be removed so its documentation too)

What points need to be improved?

For starters, a clear explanation of what the plugin does. Something like:

If the linter plugin is enabled and the file corresponds to one of
these filetypes, each time the buffer is saved, or when the `> lint`
command is executed, micro will run the corresponding utility in the
background and display the messages when it completes.

The linter plugin also allows users to extend the supported filetypes.
From inside another micro plugin, the function `linter.makeLinter` can
be called to register a new filetype. Here is the spec for the `makeLinter`
function:
Andriamanitra commented 3 months ago

But what would be the problem with making the user write bind = 'Alt-m' so that he has to write local config = import('micro/config') and config.TryBindKey('Alt-m' , 'command:goimp', true) ? Since we can simplify the use, why would I continue with the complex/verbose?

It's only "simpler" as in "less typing" but it's inconsistent with how rest of the editor and other plugins work. Most users already know how to do keybindings through bindings.json (or the bind command) so introducing another way to do the same thing is more confusing than helpful – especially if there are conflicts with the keybind.

taconi commented 3 months ago

but it's inconsistent with how rest of the editor and other plugins work

What do you mean inconsistent? The plugin is literally calling the config.TryBindKey function. When the user is configuring their formatters, they can create the bind configuration to be able to execute that formatter through a key shortcut. It is a formatter setting, as well as the filetype, cmd, etc ...

No other standard plugin gives the user a way to create keybindings, so because this function doesn't exist today does this make it inconsistent? For me it's one of centralizing the formatter configuration in the formatter.

taconi commented 3 months ago

[...] the file corresponds to one of these filetypes [...]

This is also not completely true because the domatch field changes the way the match with the filetype is made, and the os and whitelist fields don't even use the file type to match and they will also say whether a linter matches the file.

I think we should ignore the linter plugin, the pr is to add a plugin for the user to configure formatters and not to add a twin of the linter plugin. If you have any idea to improve the plugin, use the plugin itself for this.

taconi commented 3 months ago

I corrected a typo in the documentation and the pipeline failed, I think this pipeline is not very reliable.

JoeKar commented 3 months ago

I corrected a typo in the documentation and the pipeline failed, I think this pipeline is not very reliable.

Yes, unfortunately this issue was introduced by accident and is already tracked with #3220.