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

Reset transient mark mode to previous value when stopping expansion #225

Merged
merged 1 commit into from
May 14, 2017

Conversation

mallt
Copy link
Contributor

@mallt mallt commented Mar 1, 2017

This PR retains the value of transient-mark-mode before expanding, and resets transient-mark-mode to this value when stopping expansion.

When the value of transient-mark-mode contains 'only, the exchange-point-and-mark function will cons new 'only symbols to the transient-mark-mode value. When stopping expansion, this will leave the transient-mark-mode variable with several 'only symbols, causing the regular set-mark command (C-SPC) to be broken. When the value of transient-mark-mode is reset to the value before expanding, the set-mark command behaves correctly again.

This fixes #220 when shift-select-mode is active and when stopping the expansion with C-g or 0.

When copying the region after expanding however (with M-w f.ex.), the value of transient-mark-mode is not reset and the regular set-mark command is broken again. I guess interfering in all these cases would lead too far, but this PR already fixes the basic case of stopping expansion with C-g or 0.

@magnars Please feel tree to post your comments on this PR. I'm not a user of shift-select-mode myself, but since it seems to be enabled by default, I think it's important expand-region already handles the basic case.

Thanks!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 85.133% when pulling 8c3492e on mallt:reset-transient-mark-mode into d125220 on magnars:master.

@magnars
Copy link
Owner

magnars commented May 14, 2017

Thanks!

@magnars magnars merged commit 2357f1d into magnars:master May 14, 2017
@milkypostman
Copy link
Contributor

here is the problem with this change: having shift-select-mode enabled should not assume I want that behavior with expand-region.

Old behavior was that regardless of shift-select-mode, once I select something with expand-region, I can move the cursor and the transient mark stays the same.

With the new behavior I have to either hold shift, or I need disable shift-select-mode, which I had to end up doing.

@mallt
Copy link
Contributor Author

mallt commented Oct 10, 2017

Ok I see, I'll have a look at it!

@mallt
Copy link
Contributor Author

mallt commented Oct 15, 2017

To me it seems most intuitive for users of shift-select-mode to be able to further extend a region with shift-translated point motion keys after using expand-region.

If we would ignore shift-select-mode we would expect from the users of this mode to use the regular point motion keys to modify a region marked with expand-region. Wouldn't that be counterintuitive?

@milkypostman
Copy link
Contributor

milkypostman commented Oct 15, 2017 via email

@mallt
Copy link
Contributor Author

mallt commented Oct 16, 2017

The support for shift-select-mode was added with this commit based on issue #190.

I'm not a shift-select-mode user myself, but the commit seemed to introduce the expected behavior. I agree with your remarks however, it might make sense to add an extra configuration parameter to expand-region to indicate if shift-select-mode should be respected or not. I think the main problem is however that shift-select-mode is set to true by default although a lot of emacs users expect it to be disabled.

magnars added a commit that referenced this pull request Dec 18, 2017
Respecting shift-select-mode has turned out to break too many things. We're
going back to expand-region only using regular mark selection.

See comments on #190, #225, #220 and #229 for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants