Skip to content

Conversation

@CamilleScholtz
Copy link
Contributor

@CamilleScholtz CamilleScholtz commented Oct 11, 2017

I think this is better way to handle formatting than separate plugins, and it also makes it easier for users to add new formatters without duplicating code. Pretty much like the linter plugin.

Since it is possible to set options locally in settings.json I think it is more consistent and logical to enable or disable formatting for specific languages there, instead of adding a new setting for each and every formatter.

@CamilleScholtz
Copy link
Contributor Author

Thoughts on this @zyedidia?

@RangerMauve
Copy link

Do you have this published somewhere so it can be installed separately?

@zyedidia
Copy link
Member

zyedidia commented Nov 1, 2017

I'm starting to come around to this idea. I don't like directly supporting languages in default plugins unless it's an almost must-have feature but I think the case can be made here.

Another possible problem is that people might want to disable certain formatters and not others. For example Go almost requires automatic formatting but for fish or shell people might have style preferences that the formatter overrides (I'm not sure because I haven't used shell formatters) so someone might want to enable Go but not Fish formatting.

@CamilleScholtz
Copy link
Contributor Author

CamilleScholtz commented Nov 1, 2017

So I was thinking the formatter is disabled by default, and say someone wants go formatting they enable it using these local options in the config:

{
	"*.go": {
		"formatter": true
	},
}

@RangerMauve
Copy link

My main issue with built-in plugins is the customization is a lot more difficult. The linter, for example, doesn't support eslint for js, and therefore isn't useful for projects needing that. It would be better if there was a "linter runtime" which other plugins could hook into for the actual linting implementations, sort of how atom does it. This formatter is awesome, but since it's hardcoded it won't be useful for anybody doing something that's not covered. If it was an external plugin, it would at least be easier for people to modify it themselves if they needed to add something.

@CamilleScholtz
Copy link
Contributor Author

CamilleScholtz commented Nov 1, 2017

@RangerMauve
I personally think it is harder for someone to create their own plugin with all the boilerplate needed for formatting and add it it to to plugin channel than to create a PR request with ~2 new lines, just like the linter.

@RangerMauve
Copy link

@onodera-punpun I think that's more of a reason to improve the process of adding plugins to the registry than for adding more plugins into the core. (btw I love your username!)

@sum01
Copy link
Contributor

sum01 commented Nov 5, 2017

How about this? It declares the name of the formatter as an option, which is off by default. To add more formatters, you just plug it into the fmt_table and it handles the rest.

VERSION = "1.0.0"

-- Table (of tables) for lookup (keep alphabetical order plz).
-- Reported filetype (by Micro) on the left, command & args on the right.
fmt_table = {
  ["fish"] = {"fishfmt", "-w"},
  ["go"] = {"gofmt", "-s -w"},
  ["rust"] = {"rustfmt", nil}, -- no args, overwrite is default
  ["shell"] = {"shfmt", "-s -w"}
}

-- Declares the table & options to enable/disable formatter(s)
-- Avoids manually defining commands twice by reading the table
for i, value in pairs(fmt_table) do
  -- i and value[2] aren't needed here. Maybe optimize this?
  
  -- An empty table element would return nil
  if value[1] ~= nil then
    -- value[2] would return args, which we don't need here. We want the cmd string.
    if GetOption(value[1]) == nil then
      -- Disabled by default, require user to enable for safety
      AddOption(value[1], false)
    end
  end
end

function run_fmt()
  -- Prevent infinite loop of onSave()
  CurView():Save(false)
      
  local file_type = CurView().Buf:FileType()
  -- The literal filetype name (`rust`, `shell`, etc.) is the table's key
  -- [1] is the cmd, [2] is args
  local target_fmt = fmt_table[file_type]

  -- Only do anything if the filetype has is a supported formatter
  if target_fmt ~= nil then
    -- Only do anything if the specified formatter is enabled
    if GetOption(target_fmt[1]) then

      -- Load in the 'base' command (ex: `rustfmt`, `gofmt`, etc.)
      local cmd = target_fmt[1]
      -- Check for args
      if target_fmt[2] ~= nil then
        -- Add a space between cmd & args
        cmd = cmd .. " " .. target_fmt[2]
      end
      -- Add a space between the cmd & path
      cmd = cmd .. " "
      
      -- Actually run the format command
      local handle = io.popen(cmd .. CurView().Buf.Path)
      local result = handle:read("*a")
      handle:close()
      -- Reload
      CurView():ReOpen()
    end
  end
end

function onSave(view)
  run_fmt()
end

-- User command
MakeCommand("fmt", "fmt.run_fmt", 0)

Gist here https://gist.github.com/sum01/640ba8710c25469b179750069c064b11

@RangerMauve
Copy link

I think the only thing that's missing is that it needs to fetch formatter options from the settings.json, and the same changes should be done for the linter.

@sum01
Copy link
Contributor

sum01 commented Nov 6, 2017

@RangerMauve It does check if enabled here if GetOption(target_fmt[1]) then

What else would you check for in the settings.json?

PS: I have a kind of revised version of it to be able to list all formatters, but unsure if that's necessary. Something like fmt list would return all the value[1]'s in a string like fmt's supported formatters: gofmt, fishfmt, rustfmt, shfmt etc.

@RangerMauve
Copy link

Ideally you should be able to define keys for fmt_table within the config so that you could add new formatters without having to recompile micro

@RangerMauve
Copy link

Because that's the main issue with having stuff like that bundled in with Micro, if you want to add new formats you need to recompile everything, whereas if you have it in a plugin (that can be edited), or have settings in the config, people can add support for whatever environments they're working with without having to get PRs approved and recompiled.

@sum01
Copy link
Contributor

sum01 commented Nov 6, 2017

Yeah, I guess so. Maybe I'll just upload it as a stand-alone plugin once I figure out why MakeCompletion isn't working 😕

@sum01
Copy link
Contributor

sum01 commented Nov 7, 2017

This should be sufficient https://github.com/sum01/fmt-micro
It supports multiple formatters, and more can be added very easily.

@RangerMauve
Copy link

That's awesome, you should add a PR to the plugin channel so that other people will see it. https://github.com/micro-editor/plugin-channel

@sum01
Copy link
Contributor

sum01 commented Nov 7, 2017

Already did 👍

@Israel-Laguan
Copy link

If this was solved using https://github.com/micro-editor/plugin-channel please close the PR

@taconi
Copy link
Contributor

taconi commented Mar 17, 2024

#3166 implements this functionality.

@JoeKar JoeKar closed this Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants