rjsx-mode icon indicating copy to clipboard operation
rjsx-mode copied to clipboard

Indentation with tabs does not work correctly

Open matthemsteger opened this issue 5 years ago • 10 comments

After the fix in https://github.com/felipeochoa/rjsx-mode/commit/68fe4c0e0277220e04f420e1968b9d251b4b75d1 it looks like tabs are replaced by (the appropriate number of) spaces. I can dig a little further, but this appeared after this fix (which I am thankful for, it made writing JSX very difficult).

Switching to js2-mode makes things work appropriately.

Various indentation variables (I use editorconfig-emacs to set these) js2-basic-offset: 4 js-indent-level: 4 sgml-basic-offset: 4 indent-tabs-mode: t

matthemsteger avatar Sep 22 '18 19:09 matthemsteger

This is a known limitation of the indentation code. I would have thought the change happened in eae8137, though. I should add this to the README, but for now if you want tabs you can revert to the native js-mode indentation, you can use (setq-local indent-line-function 'js-jsx-indent-line) in your rjsx-mode-hook or similar.

As always, PRs welcome. I don't use tabs myself, so this is unlikely to get fixed soon. (Since I'm not familiar with how indent-tabs-mode should behave, it would mean a lot of manual reading for me to even know what the goal is).

That said, if anyone's interested, I would start by removing this binding and seeing what happened. It might be that that alone is enough.

Otherwise, I'd look into rjsx--indent-line-to-offset and its callsites, since that handles all the JSX-specific indentation. The JS indentation is delegated to js-indent-line, which it sounds like already handles tabs correctly. I think the indentation code is straightforward (basically just a switch on the various JSX pieces), but happy to help interpret if necessary.

felipeochoa avatar Sep 23 '18 14:09 felipeochoa

Ahhh, yeah, I wasnt 100% sure on the timing because at my job we use spaces and at home I prefer tabs (its a constant internal struggle).

At any rate, I will try the workaround. I wish I could quickly submit a PR, but unfortunately elisp is very alien to me so it will take some getting up to speed.

matthemsteger avatar Sep 24 '18 22:09 matthemsteger

A workaround alternative to what README suggests:

(add-hook 'rjsx-mode-hook
          (lambda ()
            (add-hook 'before-save-hook
                      (lambda ()
                        (if indent-tabs-mode
                            (tabify (point-min) (point-max))
                          (untabify (point-min) (point-max))
                          ))
                      nil
                      t)))

futpib avatar Apr 04 '19 13:04 futpib

So I had been using this for quite a while, but after the update to Emacs 27, this work around no longer works because of https://github.com/mooz/js2-mode/issues/529. I tried to see if I could create a new major mode locally that derived from rjsx-mode that set the things that were trying to be set in https://github.com/mooz/js2-mode/pull/523, but that did not work with a lot of the nice code in in rjsx-mode to make writing JSX better.

So unfortunately, not a great solution now for tabs.

  1. Use js-mode and js2-minor-mode and live with worse syntax highlighting and no extra JSX features, but correct indentation
  2. Use rjsx-mode and have no indentation really, and rely on something else (like prettier) to just clean up your code

Right now I am doing the second.

I tried to modify rjsx-mode by implementing the change in #109 but that did not work. I suspect it probably worked prior to Emacs 27 and the rewrite of everything in js-mode.

matthemsteger avatar Dec 12 '20 07:12 matthemsteger

@matthemsteger So what happens if you just remove that indent-tabs-mode binding from the code like Felipe's suggested in the first comment here?

Like https://github.com/thornjad/rjsx-mode/commit/e0a06a6622c0a3b455e62c2dbc4665e0dafaaab5 does.

dgutov avatar Jan 17 '21 01:01 dgutov

@dgutov I did try that, unless I messed it up, it did not work. That being said, I had not realized rjsx-mode had rjsx-minor-mode, so using that solves the js2-minor-mode issues with jsx it seems. So using js-mode, js2-minor-mode, rjsx-minor-mode and tree-sitter-mode seems to get pretty close to functionality and syntax highlighting, without a couple of the nice features of rjsx-mode for JSX in particular.

matthemsteger avatar Jan 17 '21 04:01 matthemsteger

I tried to remove the indent-tabs-mode binding (and the .elc file as well, to make sure it runs the modified code), and it kind of works, except that in the JSX parts it mixes tabs and spaces in order to get the 2 spaces indents (tabs are 4 spaces for me).

guillaumebrunerie avatar Jul 05 '21 18:07 guillaumebrunerie

except that in the JSX parts it mixes tabs and spaces in order to get the 2 spaces indents

That's what indentation with tabs in Emacs does. To avoid it, make sure all offsets are multiples of the tab width.

Did you expect some other behavior?

dgutov avatar Jul 05 '21 19:07 dgutov

I guess it is due to #106 then. But I did not expect some other behavior, I just mentioned it because ESLint complained a lot about "Mixed spaces and tabs", so I didn't want to simply say "it works". But you’re right that it is just the normal behavior of indentation with tabs in Emacs.

guillaumebrunerie avatar Jul 05 '21 19:07 guillaumebrunerie

Just as a followup, I usually use spaces but have been working on a project where tabs are used and so needed this. I tracked down the binding change that @felipeochoa suggested and it has been working for me so far. Just saw this issue and wanted to give feedback.

Thanks for rjsx-mode. It's been very helpful!

genovese avatar Aug 19 '22 02:08 genovese