Skip to content
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

Add mmm-indent-region #75

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

AdamNiederer
Copy link
Contributor

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.

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 has roughly equivalent function to the first one except it falls back to
mmm-indent-line when there isn't a region to narrow.
@AdamNiederer
Copy link
Contributor Author

AdamNiederer commented Oct 4, 2017

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.

mmm-region.el Outdated
(overlay-end mmm-current-overlay))
indent-function)
indent-function)))))
(if mmm-current-overlay
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But mmm-current-overlay can be outdated, can't it?

That's why we call mmm-update-submode-region down there.

Also makes mmm-indent-line-narrowed update the submode region before testing the
current overlay.
@AdamNiederer
Copy link
Contributor Author

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).

@dgutov
Copy link
Owner

dgutov commented Oct 15, 2017

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.

@AdamNiederer
Copy link
Contributor Author

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.

@dgutov
Copy link
Owner

dgutov commented Oct 16, 2017

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants