search

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.
187 stars 31 forks source link

Remove syntax table #36

Closed tummychow closed 6 years ago

tummychow commented 6 years ago

As we discussed on #35. I actually don't know how to run the tests for this repo (haven't written a lot of elisp), so I can't tell if they're passing. Let me know if there are issues.

By the way, I have a couple more commits for your consideration here. I didn't want to piggyback them into this change because they're not really related (also, they're breaking), but let me know if you're willing to accept these in another PR.

rrthomas commented 6 years ago

Thanks for this!

Sorry, I didn't spot (and can't now find) any mention of tests in #35. I'm not familiar with the tests myself, but as far as I can see, M-x eval-buffer in dtrt-indent-tests.el is sufficient. (When I do that with the current release 0.3 I get no output whatsoever, but that appears fine; eval-ing individual tests produces a reassuring nil.)

Since there have been changes since the last release (and I hadn't noticed I'd forgotten to make another), I shall make another (the tests seem to pass!) right now.

Further PRs after this one certainly welcome.

rrthomas commented 6 years ago

I've now set up Travis; could you possibly rebase on top of master and set up Travis on your fork too?

Meanwhile, I think I misunderstood what you were going for with your PR: I thought you wanted to remove the syntax table just for bash, not for all languages. I'm not sure that removing it for all languages is a good idea. Could we start with just removing it for bash (I guess having a regex that simply allows every line would be the way to do it?)?

Also, mixing dtrt-indent and SMIE sounds like it has a use, as you say, but I suspect it'll throw up problems, so could it be behind a setting that defaults to off for now, please? Also, in a separate PR.

tummychow commented 6 years ago

Restored all the syntax tables except the bash one, and removed the smie config guess change.

rrthomas commented 6 years ago

Thanks, that's great. As I say, having dtrt-indent in SMIE mode might well be good too, just ought to be toggle-able…and do bring on those other changes you mentioned.