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

Adding support for delete entire line/lines #1763

Merged
merged 3 commits into from
Oct 12, 2012
Merged

Conversation

mcorlan
Copy link
Contributor

@mcorlan mcorlan commented Oct 4, 2012

Hitting Ctrl+Y will delete the line where the cursor is or the lines for the current selection including the end of line.

@ghost ghost assigned redmunds Oct 4, 2012
@redmunds
Copy link
Contributor

redmunds commented Oct 4, 2012

This works great! My only concern is that Ctrl-Y is already used on Windows as Redo. Shift-Ctrl-Z also works as Redo on Windows, so maybe it's ok. I'll ask the rest of the core team what they think. Is there another short-cut you could use? Worst case is that this may need to be converted to an extension.

@mcorlan
Copy link
Contributor Author

mcorlan commented Oct 4, 2012

Yeah, I agree is not the best the shortcut. I would gladly use any shortcut you guys want.

@DennisKehrig
Copy link
Contributor

I vote Ctrl-Shift-L, because SciTE uses it. :)
Ctrl-Shift-D also makes sense to me. Ctrl-D could duplicate a line, Ctrl-Shift-D would in some sense be the inverse of that.

@redmunds
Copy link
Contributor

redmunds commented Oct 4, 2012

Supposedly this also conflicts with a shortcut on Mac, also. I am not sure how to determine what's the best shortcut to use, but I think this will be best if this is converted to an extension. That way, anyone can pick and choose which commands they want to load. Here are some resources that may help:

https://github.com/adobe/brackets/wiki/How%20to%20write%20extensions
https://github.com/davidderaedt/Brackets-extension-toolkit

@mcorlan
Copy link
Contributor Author

mcorlan commented Oct 4, 2012

Are you sure guys that a feature like this should be an extension? To me is just as important as Copy/Paste and so forth :). I would expect to just be there.

I know the shortcut is not easy to figure out. The ones used by Eclipse or Idea for the same feature are already used by Brackets for something different.

Are you guys happy with "Ctrl-Shift-D"?

@redmunds
Copy link
Contributor

redmunds commented Oct 4, 2012

As much as I appreciate you contributing to Brackets, this is not a feature that I, personally, use. I would rather select the lines and hit delete. That way I know exactly what is being deleted, I don't have to remember another shortcut, and the editor code is slightly smaller. The more commands taht are implemented as extensions, the more choice everyone has to customize their editor.

Another part of the problem is that there are just not enough shortcuts for every feature. We really need to start a wiki page with a table of all the shortcuts so we can easily see what we have in both core and extensions.

You are welcome to start a discussion in the brackets-dev forum (https://groups.google.com/forum/?fromgroups#!forum/brackets-dev) to see if you can get a consensus. Most of the core brackets team is in Europe for the Create the Web tour, and won't be back for a week, so I won't be able to poll everyone for awhile, anyway.

@GarthDB
Copy link
Member

GarthDB commented Oct 4, 2012

I think I would use it, and it as useful as duplicate line.

Garth Braithwaite
Sent with Sparrow (http://www.sparrowmailapp.com)

On Thursday, October 4, 2012 at 11:53 AM, Randy Edmunds wrote:

As much as I appreciate you contributing to Brackets, this is not a feature that I, personally, use. I would rather select the lines and hit delete. That way I know exactly what is being deleted, I don't have to remember another shortcut, and the editor code is slightly smaller. The more commands taht are implemented as extensions, the more choice everyone has to customize their editor.
Another part of the problem is that there are just not enough shortcuts for every feature. We really need to start a wiki page with a table of all the shortcuts so we can easily see what we have in both core and extensions.
You are welcome to start a discussion in the brackets-dev forum (https://groups.google.com/forum/?fromgroups#!forum/brackets-dev) to see if you can get a consensus. Most of the core brackets team is in Europe for the Create the Web tour, and won't be back for a week, so I won't be able to poll everyone for awhile, anyway.


Reply to this email directly or view it on GitHub (#1763 (comment)).

@mcorlan
Copy link
Contributor Author

mcorlan commented Oct 4, 2012

@redmunds

Forcing this feature on you as a core feature was not my intention at all. If this is how it came across I apologize. I used my personal experience as a starting point for a healthy debate. Ultimately I am not the one to decide. You guys have the last word.

Having said that I don't believe that we should be afraid that we end up with too many shortcuts. If this would be true for the vast majority of programers then I am afraid that VIM would have been gone long time ago. VIM is completely useless without shortcuts. I guess people will use those shortcuts that make sense to them and ignore the others. What are those useful or not it is hard to say without doing a statistical research among your product users.

I was thinking today about the shortcuts discoverability problem. You are right we should do something about. Ideally I think we should have an API to declare shortcuts in such a way that a "live" list with all the available shortcuts (from core features or extensions) is generated at runtime and makes easy for developers to see what is available. This would be the first step, the second one would be to let them change the shortcuts.

Thank you for taking your time to review my pull request and expressing your concerns. I think in the end we all want to get the best of the best possible code editor :)

@DennisKehrig
Copy link
Contributor

I use it all the time, too. It works regardless of where the cursor is in that line and whether anything is selected or not, while all first-select-then-delete based methods are just that extra bit more involved.

I think in the long run we need a keyboard shortcut manager. Not having conflicting shortcuts is one thing, but some shortcuts don't make sense on a German keyboard, for instance. While "[" is a single key stroke on an English keyboard, it's AltGr-[ (Ctrl-Alt-[) on a German one, so a shortcut like Ctrl-[ doesn't really make a lot of sense on a German keyboard.

In the meantime a Wiki page like @redmunds suggests would definitely be useful, I think there's one extension out there that uses Ctrl-Shift-E - just like the extension manager. That should be avoided.

@redmunds
Copy link
Contributor

MIhai,

The team wants to pull this into the core code. Shortcut "Ctrl-Shift-D" is available and should be ok.

In addition to having a shortcut key, it should be in the Edit menu so that it's discoverable. I didn't notice before, but the shortcut should not be added to the codeMirrorKeyMap. You can assign the shortcut at the same time you add the menu item. Take a look in Menu.js.

Thanks,
Randy

Changed the code to be exposed in the Edit Menu;
Shortcut chaged to Cmd-Shift-D (Ctrl-Shift-D);
@mcorlan
Copy link
Contributor Author

mcorlan commented Oct 11, 2012

Hey Randy,

I changed the code accordingly to your suggestions. I hope it is OK the way I exposed/register to the menu.

Thanks for your patience!

cheers,
Mihai

@redmunds
Copy link
Contributor

@mcorlan,

This looks good. The last thing you need to do is the accept the Contributor's License Agreement: http://dev.brackets.io/brackets-contributor-license-agreement.html. Let me know when that's done.

Thanks,
Randy

@mcorlan
Copy link
Contributor Author

mcorlan commented Oct 12, 2012

Done :)

Thanks Randy! It feels awesome to finally land a piece of code/functionality!

@redmunds
Copy link
Contributor

Looks good. Merging. Welcome to the club, Mihai! Thank you for your contribution.

redmunds added a commit that referenced this pull request Oct 12, 2012
Adding support for delete entire line/lines
@redmunds redmunds merged commit 5eddb2a into adobe:master Oct 12, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants