dtrt-indent icon indicating copy to clipboard operation
dtrt-indent copied to clipboard

Several improvements in file local variable usage

Open ambihelical opened this issue 7 years ago • 13 comments

The commits first to last:

  • make sure file local variables are handled properly by performing the core processing after these variables have been processed, instead of before.
  • Add ability to set related variables when any mode derived from a particular major mode is in effect
  • add new variable dtrt-indent-force-offset, which can be used to force the offset to be a particular value without affecting any of the other processing. This is intended to be used as a file local variable. This addresses #46
  • Fixed the handling of file local indent-tabs-mode setting so that it overrides dtrt-indent's analysis, which seemed to be missing from the code (dtrt-indent-explicit-tab-mode was set, but not used).

ambihelical avatar Feb 23 '19 19:02 ambihelical

Great stuff. Is this now ready to be merged as far as you're concerned?

rrthomas avatar Feb 23 '19 21:02 rrthomas

I think so. If you are reluctant, I can give it another week of testing at my day job. I did make some minor changes during the week, and I'm not in any rush in any case.

ambihelical avatar Feb 24 '19 01:02 ambihelical

Another week would be great if that's OK by you. I'll also try your branch.

rrthomas avatar Feb 24 '19 19:02 rrthomas

OK to ping me in a week? Many thanks in any case!

rrthomas avatar Feb 24 '19 20:02 rrthomas

I just noticed that your branch isn't working for me, in what looks like a fairly basic case: I have c-basic-offset set to 2, I visit a file that dtrt-indent evaluates (correctly) as having an indent of 4, and c-basic-offset is not adjusted. There are no file-local or directory-local overrides in this case.

Sorry, I don't have time to debug this right now, but thought you'd want to know.

rrthomas avatar Feb 26 '19 21:02 rrthomas

OK, I'll look into it.

ambihelical avatar Feb 27 '19 00:02 ambihelical

So far I'm not able to reproduce with a file with 3-space indent (no hard tabs), no .dir-locals.el, and me setting c-basic-offset to 8 in my init.el for cc-mode. It correctly detects offset of 3. What does dtrt-indent log in your case?

ambihelical avatar Feb 27 '19 04:02 ambihelical

Nothing is logged. If I run dtrt-indent-diagnosis, it's getting it right:

Guessing offset for /home/rrt/Software/smite/src/storage.c

Elapsed time for analysis: 0.005 seconds

Total relevant lines: 121 out of 269 (limit: 5000)

Histogram:

    74x   4 spaces
    43x   8 spaces
     4x  12 spaces

Analysis:

  offset 2 works for 100.00% of relevant lines, matching 3 distinct offsets - merged with offset 4 (0.00% deviation, limit 20.00%)
  offset 4 works for 100.00% of relevant lines, matching 3 distinct offsets - CONSIDERED
  offset 8 works for  35.54% of relevant lines, matching 1 distinct offsets - CONSIDERED
  offset 3 works for   3.31% of relevant lines, matching 1 distinct offsets - merged with offset 6 (0.00% deviation, limit 20.00%)
  offset 6 works for   3.31% of relevant lines, matching 1 distinct offsets - CONSIDERED
  offset 5 works for   0.00% of relevant lines, matching 0 distinct offsets - rejected: too few distinct matching offsets (1 required)
  offset 7 works for   0.00% of relevant lines, matching 0 distinct offsets - rejected: too few distinct matching offsets (1 required)

Summary:

  Best guess is offset 4 with 100.00% matching lines (80.00% required)
  Hard tab percentage: 0.00% (0 lines), -100.00% superior to soft tabs (threshold 50.00%)
  Soft tab percentage: 100.00% (121 lines), inf% superior to hard tabs (threshold 300.00%)

Conclusion:

  Guessed offset 4 with 100% confidence.
  Change indent-tab-setting: yes, to nil

However, it doesn't alter anything; maybe it thinks my setting is already 4 for some reason?

With current head, I get the same output from dtrt-indent-diagnosis, and the following logged:

Note: c-basic-offset adjusted to 4

I should also note that my c-basic-offset's global value is actually set-from-style.

rrthomas avatar Feb 27 '19 11:02 rrthomas

Very odd. I'll look at the code further when I get a chance.

ambihelical avatar Feb 27 '19 16:02 ambihelical

I don't see anything that would explain the different behaviours we are seeing. I would check that dtrt-indent-post-local-variable-setup runs for the buffer, maybe the hooks aren't working the way I thought for some reason. Also, what does your dtrt-indent setup look like?

ambihelical avatar Feb 28 '19 06:02 ambihelical

All my dtrt settings are customizations:

'(dtrt-indent-global-mode t) '(dtrt-indent-min-hard-tab-superiority 50.0) '(dtrt-indent-min-indent-superiority 90.0)

Checking, I find that dtrt-indent-post-local-variable-setup is added to hack-local-variables-hook. However, when I visit a file, indeed it does not appear to be run (I added a call to message to confirm this).

I tried disabling dtrt-indent-global-mode, and then manually running M-x dtrt-indent-mode after visiting a C file, but this didn't cause the hook to run either. I can confirm that dtrt-indent-mode is indeed set in the buffer in question, and after adding another call to message in dtrt-indent-mode, that that function is being run, and further that the call to add-hook is being run. But of course by then the file is already loaded, so the hook is not run…

So to summarize, if I run dtrt-indent-mode manually, after a buffer is already visiting a file, presumably this is too late, as hack-local-variables-hook has already been run. It seems that for some reason with dtrt-indent-global-mode, the hook is not being run; maybe it's too late then too?

rrthomas avatar Mar 01 '19 17:03 rrthomas

That analysis seems correct. I ran dtrt-indent-mode from a prog-mode hook, which I guess has the right timing for the code in the PR. I'll revise it so it works correctly for that case, for manual invoke and for the global mode. Not sure how to do that yet, but I guess I'll learn something in the process.

ambihelical avatar Mar 02 '19 19:03 ambihelical

Great stuff, looking forward to it!

rrthomas avatar Mar 02 '19 19:03 rrthomas