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

Add mmm-indent-region

Open AdamNiederer opened this issue 7 years ago • 5 comments

Provides a function which can indent all submodes partially in the region, even if they don't take up an entire line or have special indentation needs (e.g. narrowing).

Also adds a somewhat less convoluted implementation of mmm-indent-line-narrowed, which works similarly to the first one except it falls back to mmm-indent-line when there isn't a region to narrow.

AdamNiederer avatar Oct 04 '17 15:10 AdamNiederer

This fixes one last problem I was having with indenting regions which overlapped but did not fully contain a submode overlay. It also causes submodes which take up less than one line or start in the middle of a line to e indented properly. The change to mmm-indent-line-narrowed makes it work better with mmm-indent-region, while having the same side effects.

AdamNiederer avatar Oct 04 '17 15:10 AdamNiederer

Apologies for the delay.

I've moved both commands to mmm-cmds, made mmm-indent-line-narrowed update the submode before proceeding, and added autoloads for both commands. These changes fix the remainder of the problems I've been facing in vue-mode (at least on my current test cases).

AdamNiederer avatar Oct 13 '17 17:10 AdamNiederer

Looking at the description again, I have to ask:

This fixes one last problem I was having with indenting regions which overlapped but did not fully contain a submode overlay.

Does that mean that the line indentation function still performs incorrectly in certain cases? That seems like it should be fixed first.

indent-region-function can be handy, but I don't think it should be used for anything other than improving performance of reindenting a huge region. Not correctness.

dgutov avatar Oct 15 '17 21:10 dgutov

Does that mean that the line indentation function still performs incorrectly in certain cases? That seems like it should be fixed first.

Yes, there was a bug in mmm-indent-line-narrowed on master which was fixed in this PR. Sorry for letting that slip through :(.

I agree that an indent-region-function should only be used for perf in a standard buffer, but mmm-mode provides some interesting challenges when deciding what a "line" is.

Suppose we have the toy mode:

(require 'mmm-mode)

;;;###autoload
(define-derived-mode kek-mode prog-mode "kek"
  (mmm-add-classes `((c :submode js-mode :front "ccc" :back "ddd")))
  (mmm-add-mode-ext-class 'kek-mode nil 'c))

(provide 'kek-mode)

And the toy buffer:

ccc let a = 2 ddd test ccc function  (a, b) { }
let a = 2 ddd hello

What do we do if the user presses TAB? Do we indent all of the modes, the mode which point resides in, or just the parent mode? What if the user selects the entire line and then presses TAB?

The current behavior is mmm-indent-line-narrowed and mmm-indent-line will only indent the mode in which point resides, or the first mode on the line. mmm-indent-region allows authors to indent each submode contained in the region in accordance with its rules as well, even if it doesn't start on a newline.

Neither is more "correct" imo but allowing us to indent multiple submodes per line in accordance with their individual rules has applications where the submodes don't exactly line up with \n, and is a good pairing to mmm-indent-line-narrowed.

AdamNiederer avatar Oct 16 '17 03:10 AdamNiederer

Do we indent all of the modes

Obviously, you will. But by the rules of the mode that the position at the indentation belongs to.

Where is point in this example anyway?

What if the user selects the entire line and then presses TAB?

That's the same as just pressing TAB. And anyway, if the point is on the first line, how would you indent the kek submode in there?

dgutov avatar Oct 16 '17 10:10 dgutov