biomejs / biome-zed

Biome extension for Zed
https://biomejs.dev
MIT License
169 stars 6 forks source link

Formatting inconsistencies #36

Closed Marcisbee closed 3 months ago

Marcisbee commented 3 months ago

There still are differences in formatting in zed vs cli after updating to 0.146.3. I am not sure if this is zed issue or biome issue.

Here's a simple reproduction for this: https://github.com/Marcisbee/zed-biome-reproduction-1

Have I configured something wrong or is there a bug somewhere?


Also can try adding this to index.ts file:

const something
  = 123;

Cli says it's an error, but running formatter from zed, does nothing with this.

Biome: 1.8.3 Zed: 0.146.3 (also happens in Zed Preview 0.147.0)

settings.json

{
  ...
  "lsp": {
    "biome": {
      "settings": {
        "require_config_file": true,
        "config_path": "<path>/biome.json"
      }
    }
  },
  "formatter": {
    "language_server": {
      "name": "biome"
    }
  },
  "code_actions_on_format": {
    "source.organizeImports.biome": true,
    "source.fixAll.biome": true
  }
}
luckydye commented 3 months ago

Do you actually have "\" in your config_path? Try removing the whole "lsp" settings and see if it works. Also you can check in the command panel -> "debug: open language server logs" if the biome lsp is actually running or failed to start.

Marcisbee commented 3 months ago

Indeed, removing config_path now seems to work(ish). There still are some weird cases. I updated reproduction with biome config:

    "formatter": {
        "enabled": true,
        "indentStyle": "space"
    },
    "javascript": {
        "formatter": {
            "quoteStyle": "single"
        }
    },

And this config doesn't seem to be respected when doing formatting from zed. Cli now says there are issues, but zed formatter does leave spacing with tabs and double quotes for this example:

try {
    // Something here
    const a = "test";
} catch (_e) {}

Current settings.json

{
  ...
  "lsp": {
    "biome": {
      "settings": {
        "require_config_file": true
      }
    }
  },
  "formatter": {
    "language_server": {
      "name": "biome"
    }
  },
  "code_actions_on_format": {
    "source.organizeImports.biome": true,
    "source.fixAll.biome": true
  }
}
Marcisbee commented 3 months ago

With this configuration ("require_config_file": false) it seems to be working just fine:

  "lsp": {
    "biome": {
      "settings": {
        "require_config_file": false,
        "config_path": "<path>/biome.json"
      }
    }
  },

But then it also runs in projects where config file is not defined, right?

luckydye commented 3 months ago

For the record, the "\" is a placeholder for the path to where the biome.json file is relative to the root, this should no actually be in your settings.json.

luckydye commented 3 months ago

But then it also runs in projects where config file is not defined, right?

Correct. Tho there may be a bug with this setting, can you check if there are any errors logged in your language server or zed logs? :)

Marcisbee commented 3 months ago

It seems like it's working correctly now without config_path and "require_config_file": true.

I just had to restart zed for this to take effect.. 🤦

Seems like this was a user error, tho it would be nice for this extension to detect this config change without reloading workspace.