search

felipeochoa / rjsx-mode

A JSX major mode for Emacs
https://github.com/felipeochoa/rjsx-mode
MIT License
639 stars 32 forks source link

dont override user's tab preference #109

Open thornjad opened 5 years ago

thornjad commented 5 years ago

As mentioned in #85, eae8137 introduced a local override to indent-tabs-mode which (unexpectedly to the user) switches to spaces to indent. By simply removing that pair, indentation now works the way I expect. I didn't observe any other changes to indentation, though I would appreciate someone else who is more familiar with this code taking a look as well.

Fixes #85

felipeochoa commented 5 years ago

Tests please!

thornjad commented 5 years ago

Oh yes, those would be a good idea! Do you know offhand how to fix the existing tests so they'll run? I only have limited experience with ert.

-------- Original Message -------- On Jun 29, 2019, 9:32 AM, Felipe wrote:

Tests please!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

felipeochoa commented 5 years ago

According to Travis all tests are passing on this branch. ert is not super hard to use -- start a fresh Emacs instance, load-file the various testing files (the order matters so that require succeeds I think), and then do M-x ert. As you add/change tests, eval the test definitions and re-run ert. That should get you going :)

thornjad commented 5 years ago

No I mean the tests are not even running, even on Travis. It is erroring out with:

$ make test EMACS=${EMACS}
batch -Q -L . -l rjsx-mode.el -l js2-tests.el -l rjsx-tests.el\
  -f ert-run-tests-batch-and-exit
batch accepts no parameters
make: [test] Error 1 (ignored)

For whatever reason that error is returning 0, making Travis think it passed. That's the issue I'm referring to.

felipeochoa commented 5 years ago

Ah I see. No clue what's going on with Travis. To run them locally after a clone do make install-deps followed by EMACS=emacs make test

thornjad commented 5 years ago

Oh odd, when I submitted this PR I was getting the same error locally, but now it's running fine. Perhaps I did it wrong before.

In any case, I'll make up some tests!

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Saturday, June 29, 2019 11:49 AM, Felipe notifications@github.com wrote:

Ah I see. No clue what's going on with Travis. To run them locally after a clone do make install-deps followed by EMACS=emacs make test

—

You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub, or mute the thread.[https://github.com/notifications/beacon/AEE3WD27LZBVN6P4FTDK3IDP46HA5A5CNFSM4H3LHC3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY34JBY.gif]

thornjad commented 2 years ago

Hi all, sorry this completely fell through the cracks for me. Unfortunately, in the time since I've made this PR, I no longer work at a place that requires tabs in everything, and I no longer work in React very often so I've switched back to js-mode.

I won't be getting back to writing tests here, but I more than welcome anyone else writing them, someone else taking over this PR, or closing and letting someone else make the same change I did, whatever works best for the project!

felipeochoa commented 2 years ago

I'll leave it open in case anyone wants to pick it up