-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dont override user's tab preference #109
base: master
Are you sure you want to change the base?
Conversation
Tests please! |
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](#109?email_source=notifications&email_token=AEE3WD6H7KEV25ACJCSNH53P45XBFA5CNFSM4H3LHC3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3Z3BI#issuecomment-506961285), or [mute the thread](https://github.com/notifications/unsubscribe-auth/AEE3WDY3KTTN3HFZ2JB4XUTP45XBFANCNFSM4H3LHC3A).
|
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 :) |
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.
|
Ah I see. No clue what's going on with Travis. To run them locally after a clone do |
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 ***@***.***> 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]
|
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 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! |
I'll leave it open in case anyone wants to pick it up |
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