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

DeleteLine does not delete selected lines #3328

Open
Neko-Box-Coder opened this issue May 31, 2024 · 8 comments · May be fixed by #3335
Open

DeleteLine does not delete selected lines #3328

Neko-Box-Coder opened this issue May 31, 2024 · 8 comments · May be fixed by #3335

Comments

@Neko-Box-Coder
Copy link
Contributor

Description of the problem or steps to reproduce

Where there are multiple lines selected (i.e. line 1 to 10), calling DeleteLine should delete all the selected lines, not just the line the cursor is on (i.e. line 10)

Specifications

Commit hash: e9bd1b3
OS: Linux
Terminal: WezTerm

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 3, 2024

That is how Cut (Ctrl-x) already works: if there is a selection, it deletes the selection, otherwise it deletes the current line, like DeleteLine does.

BTW I don't quite like the way how it is implemented. The Cut action internally checks if there is a selection and either deletes the selection or the current line. I think it would be better if it only deleted the selection and Ctrl-x was bound by default not to Cut but to Cut|CutLine (so that if the user doesn't want Ctrl-x to delete the current line if there is no selection, the user can disable this behavior by binding Ctrl-x to Cut instead of Cut|CutLine).

@Neko-Box-Coder
Copy link
Contributor Author

Neko-Box-Coder commented Jun 3, 2024

I think what you are saying is similar but a different issue, possibly an incorrect behavior from Cut. If Cut does what you described ( I haven't checked the code yet ), we can probably just remove the non selection logic and default ctrl-x to Cut|CutLine. That's in fact my current bindings funny enough.

But anyway, that's another issue as I said (which could potentially be changed together).
What I was saying is that DeleteLine function (or even CutLine, CopyLine) should respect selections and delete the lines where and when there's a selection. This is how it works on other editors and I am still struggling to get used to it 😅

I can probably raise a PR for this (and also the issue you mentioned) but that will be after I sorted out the other PRs I have since I am already spending quite a bit of time on other PRs.

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 3, 2024

My point is that if we do what you suggest, i.e. make DeleteLine respect selections, then Ctrl-k will do the same thing as Ctrl-x, so then the question is why do we need Ctrl-k (and DeleteLine) at all.

Ah, I've just realized that DeleteLine is not the same thing as CutLine, since it doesn't copy the line to the clipboard. (I'm too used to using Ctrl-x for deleting lines, even when I don't need to copy them.)

So, yeah, it makes sense to make DeleteLine respect selections (like DuplicateLine does).

Question: if the selection does not fully cover its first and/or last line, should DeleteLine delete just the selection itself, or the rest of its first and last lines too? Which behavior is more useful?

From a pure aesthetics/consistency perspective it seems better if it will delete just the selection: 1. DuplicateLine also duplicates just the selection. 2. I've checked that in nano Ctrl-k also deletes just the selection (haven't checked other editors though).

@Neko-Box-Coder
Copy link
Contributor Author

I understand it is good to have consistency but if DeleteLine just deletes the selection, then it is doing the same job as Delete, which I could have just used Delete.

My personal opinion would be it should also delete the first line and last line since that is doing what it says, deleting lines.

However, that's just my own opinion. But I think it should at least reacts to selection(s), the action is up for debate and I don't mind other people joining to vote/provide opinions on what they want.

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 3, 2024

Yeah, good point. Now it seems more useful to me if it deletes the whole first and last lines. (DuplicateLine is a slightly different case.)

@Gavin-Holt
Copy link

Gavin-Holt commented Jun 7, 2024

Hi,

You can use Lua to achieve the required deletion. Bind the following to a keyboard shortcut of your choice:

    "YourKey": "lua:initlua.delsel,SelectLine,Delete",

I keep all my Lua in one file init.lua and the delsel function is very simple:

function delsel(Current)
-- This avoids deleting EOL when deleting partially selected lines
    if Current.Cursor:HasSelection() then
        Current.Cursor:DeleteSelection()
    end
end

This will delete all partly selected lines, or the current line if no selection.

Kind Regards Gavin Holt

PS I am struggling to achieve similar for duplicate line as my solution is now broken - see #3110.

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 7, 2024

You can use Lua to achieve the required deletion. Bind the following to a keyboard shortcut of your choice:

    "YourKey": "lua:initlua.delsel,SelectLine,Delete",

This is a smart trick, but it doesn't always work correctly. If the last line of the selection contains the whole line, this will also wrongly remove the next line below the selection.

...I think the behavior of DeleteLine should be fixed in micro anyway. The existing behavior is, among other things, confusing: DeleteLine deletes just the "current line", but when there is a selection, the cursor is not displayed, so the user doesn't know what is the current line.

I'm gonna implement a fix, it should be trivial.

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 9, 2024

Implemented #3335 which makes DeleteLine respect selections, among other things.

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 a pull request may close this issue.

3 participants