Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Selected characters on single line cannot be replaced with tab character #7416

Closed
lkcampbell opened this issue Apr 4, 2014 · 15 comments
Closed
Assignees

Comments

@lkcampbell
Copy link
Contributor

OS: Mac OSX Mavericks

Brackets Version: sprint 38 development build 0.38.0-0 (master 575bf97)

Repro Steps:

  1. Make a selection of characters on a single line.
  2. Hit the Tab key.

Expected Results:
The selected characters should be replaced with a Tab or Tab equivalent of space characters.

Observed Results:
The selected characters remain. A Tab, or Tab equivalent of space characters, is insert before the selected characters.

Other Notes::
This appears to be a regression of issue #3723.

@lkcampbell lkcampbell changed the title Selected character on single line cannot be replaced with tab character Selected characters on single line cannot be replaced with tab character Apr 4, 2014
@njx
Copy link

njx commented Apr 4, 2014

Oops. We actually intentionally changed this behavior as part of the multicursor stuff to match what some other editors do (though notably Sublime behaves as you suggest). This was discussed in some comments in #7022; I think @redmunds and I both preferred the "insert a tab" behavior rather than "replace with a tab", and we didn't remember that we'd explicitly implemented the "replace" behavior for #3723.

I think we generally felt that it was uncommon for people to want to replace arbitrary characters with a tab, but clearly you run into this case often :) Can you describe the use case where you want the replace behavior?

@lkcampbell
Copy link
Contributor Author

@njx, just FYI, the original discussion thread is here:

https://groups.google.com/forum/#!topic/brackets-dev/6vkT2LR9Wo0

Ironically, you may notice one of my first suggestions in that thread to solve the problem was to use User Preferences that we didn't have then -- but we do now :).

I think one of my most common use cases is lining up indentations of incorrectly indented code lines. Also, lining up my variable declarations and assignments, or make code lines line up properly in table form. A good example might be the document comments I put into Commands.js a while ago. Makes it easy lining it up in table form.

So, perhaps it could be said that I only need it when I am selecting whitespace. But I'm not 100% sure because it's more of a "feel" thing than anything else, so I could be missing some cases.

I think the other argument is that is it strange that Tab is the only character that doesn't replace a range of selected characters when you type it. Although it could be argued that Tab is a specialized character as well. I think we made a good argument for this behavior for selections that span multiple lines but not for a selection of characters on a single line. Shouldn't matter with multiple selections either. Collections of single line selections should still act the same in each item in the collection.

Every text editor I have looked at behaves the way that I present in this issue: Sublime Text, Text Wrangler, PS Pad (in Windows), TextEdit. Granted, I haven't done a thorough search, but I haven't been able to locate an editor that acts the way that Brackets acts now with the tab key.

However, I think all of this comes down to personal preferences in the end, which is why we should maybe consider making it a User Preference. No idea what we should call it though.

@pthiess
Copy link
Contributor

pthiess commented Apr 13, 2014

I think this needs a group review - as @lkcampbell I'd say this may be a reasonable user preference. That said I want to hear additional opinions.

@redmunds
Copy link
Contributor

I think this is related to #6877, so maybe a single preference to control both.

@lkcampbell
Copy link
Contributor Author

Three questions that should be considered in your review:

  1. Should it be a User Preference?
  2. If it is a User Preference, what should be the default behavior be?
  3. If it is a User Preference, what do we call it and/or how do we describe it in such a way that users can really understand what is does?

My recommendations:

  1. Yes.
  2. The behavior we had before these changes were made.
  3. No good ideas, sorry.

@redmunds
Copy link
Contributor

After more thought, maybe these should not be combined.

  • The indent case seems to be "indent selection" vs. "replace selection with Tab char"
  • The outdent case seems to be "outdent line" vs. "outdent selection"

@njx
Copy link

njx commented Apr 14, 2014

I'm fine just going back to the behavior @lkcampbell wants, given that it doesn't really seem like any other editors have the behavior we just changed to. @redmunds any objections?

@njx njx self-assigned this Apr 14, 2014
@redmunds
Copy link
Contributor

@njx We need to keep existing behavior for #6877, so I still think it needs to be a preference. I'm ok with making the old behavior the default.

@lkcampbell
Copy link
Contributor Author

@njx, since I have a vested interest in this issue, can I take it over from you? Just reassign it to me if it is okay.

@njx njx assigned lkcampbell and unassigned njx May 1, 2014
@njx
Copy link

njx commented May 1, 2014

Please do! Thanks for volunteering. The relevant code is in Editor._handleTabKey() and the helper functions it calls, _autoIndentEachSelection() and _addIndentAtEachSelection(), though I haven't thought through exactly which of those would be affected by the desired fix for this bug. I wrote that code fairly recently so should be able to answer questions about it if you have any.

@lkcampbell
Copy link
Contributor Author

@njx or @redmunds, any suggestions on what we call this preference? I understand what the components are and how they should behave, but I don't have an easily understood name for the behavior/preference.

@redmunds
Copy link
Contributor

@lkcampbell @njx How about tabReplacesSelection ?

@lkcampbell
Copy link
Contributor Author

@redmunds, that sounds good to me. The only exception is if the selection contains one or more newline characters, in which case the tab key maps to indent line.

One minor point on this. If I have a multiple selections and some have newline characters and some don't, do I replace them all with tabs, or indent them all, or do an appropriate mix of these two options?

cc: @njx

@redmunds
Copy link
Contributor

exception is if the selection contains one or more newline characters

Good point, so maybe tabReplacesSingleLineSelection.

If I have a multiple selections and some have newline characters and some don't, do I replace them all with tabs, or indent them all, or do an appropriate mix of these two options?

Good question. It would be good to check to see what some other editors do.

With multiple cursors/selections in general, we usually let commands operate on each cursor/selection individually. For example, if you have 1 cursor on a line of code and another on a line comment and execute toggleComment, the line of code is commented and the comment is removed from the other line.

@lkcampbell
Copy link
Contributor Author

I think I cared about this issue at one time...since then I changed my mind and my muscle memory.

Closing, for closure sake.

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

No branches or pull requests

4 participants