jscheid / dtrt-indent

A minor mode that guesses the indentation offset originally used for creating source code files and transparently adjusts the corresponding settings in Emacs, making it more convenient to edit foreign files.
186 stars 31 forks source link

web-mode support doesn't work well for heterogeneous indent types #66

Open thomasjm opened 1 year ago

thomasjm commented 1 year ago

I have a codebase that uses different values for different web-mode indentation variables within .tsx files. For example:

(setq web-mode-code-indent-offset 2)
(setq web-mode-markup-indent-offset 4)

Here's some code written this way:

import Menu from "@mui/material/Menu";
import MenuItem from "@mui/material/MenuItem";
import { useCallback } from "react";

export default function FooBarMenu() {
  const fooCallback = useCallback(() => {
    console.log("foo");
  }, []);

  return (
    <Menu label="Foo"
          disabled={false}>
        <MenuItem className=""
                  onClick={fooCallback}>
            Foo
        </MenuItem>
    </Menu>
  );
}

When I run dtrt-indent-diagnosis, it says "Guessed offset 2 with 100% confidence" and sets all my variables to 2. No doubt because it expects them all to be the same; see here.

Unfortunately this is probably hard to fix because it requires a more detailed understanding of JSX/TSX files.

rrthomas commented 1 year ago

This is a challenge indeed! I think the fundamental problem here is that dtrt-indent-mode has no idea of different indentations within the same file. As you say, it would need a more detailed understanding of JSX/TSX files for this case, but more fundamentally the whitespace scanning functionality would need the ability to perform multiple diagnoses, perhaps based on recognizing, effectively, different major modes within a given file/buffer. (I'm thinking that the same model would apply to the old mmm-mode and similar things.)

I don't know how much sense it would make to try to add this functionality to such a general-purpose mode as dtrt-indent-mode; this feels like high-hanging fruit! In most cases it's probably going to be easier to have a .dir-locals.el or .editorconfig for a project that sets the required variables.

thomasjm commented 1 year ago

In most cases it's probably going to be easier to have a .dir-locals.el or .editorconfig for a project that sets the required variables.

I had been setting them in a major mode hook, but the trouble is that Doom Emacs runs the indent detection after those hooks, leading to a confusing situation where my settings got blown away. I had to track down what was changing it by grepping for the message "Note: ... adjusted to 2" which appeared in the messages buffer. Wish that instead of "Note:" it began with "dtrt-indent:" btw :)

rrthomas commented 1 year ago

You can turn off dtrt-indent-mode in the same major mode hooks.

thomasjm commented 1 year ago

I suspect that won't work in Doom -- in case it helps anyone, the Doom solution was

(add-to-list 'doom-detect-indentation-excluded-modes 'web-mode)
(add-to-list 'doom-detect-indentation-excluded-modes 'typescript-tsx-mode)