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

Feature: New command - Delete Duplicate Lines #119480

Merged
merged 2 commits into from Oct 27, 2021

Conversation

codeclown
Copy link
Contributor

Description

VS Code implements a set of useful commands to mutate lines in the current selection(s), e.g.

  • Sort Lines Ascending
  • Sort Lines Descending

This PR implements a new command:

  • Delete Duplicate Lines

Screenshot:

image

This is useful when mass-editing lines of text. For me personally it's probably the last thing that I miss from Sublime Text, as a converted VS Code user.

Open questions

  • Is this feature mergeable to VS Code, or should it be separated as an extension instead? I think it'd make sense to have in the core, to accompany the other line mutating commands.
  • Does the selection handling in this PR look okay? This is my first PR to this codebase and I'm not 100% confident about that part.

If anything needs adjusting, I'm happy to alter the PR. Please do let me know.

@ghost
Copy link

ghost commented Mar 22, 2021

CLA assistant check
All CLA requirements met.

@NotWearingPants
Copy link
Contributor

NotWearingPants commented Mar 23, 2021

I think the logic should instead be: if there is one selection, delete duplicate lines like you did, but if there are multiple selections, delete duplicate selections.
IMO this handles more use cases than what you've done with multiple selections (e.g. if I select all the words I can remove duplicate words), which is equivalent to running the command on each selection separately.

Also, this would probably be closed as an "extension-candidate", which makes sense but doesn't justify having sorting commands built-in while this isn't.
Here's an extension written by someone actually in the VSCode team that does what you want - https://marketplace.visualstudio.com/items?itemName=Tyriar.sort-lines
Personally I think VSCode has no underlying logic to deciding what's built-in and what isn't, I'd vote to either having this command added or the sorting commands removed as an extension-candidate.

Btw PRs need to have an issue associated with them, I'm assuming a really old issue already exists but I couldn't find any

@codeclown
Copy link
Contributor Author

I think the logic should instead be: if there is one selection, delete duplicate lines like you did, but if there are multiple selections, delete duplicate selections. IMO this handles more use cases than what you've done with multiple selections (e.g. if I select all the words I can remove duplicate words), which is equivalent to running the command on each selection separately.

I based the behaviour on Sublime Text. It's also consistent with the other built-in commands, which are dealing with lines, not selections. I also think it'd just be confusing to the user if the command switched between deleting lines and deleting selections based on the amount of selections.

From personal experience, I don't recall ever needing a "delete duplicate selections" action. If there's need for it then that could just be implemented as a separate command.

Also, this would probably be closed as an "extension-candidate", which makes sense but doesn't justify having sorting commands built-in while this isn't. [...]

Yep, I agree with this, also listed this as a question above. Will leave it to project stakeholders to decide.

@alexdima alexdima added this to the April 2021 milestone Mar 23, 2021
@NotWearingPants
Copy link
Contributor

It's also consistent with the other built-in commands, which are dealing with lines, not selections. I also think it'd just be confusing to the user if the command switched between deleting lines and deleting selections based on the amount of selections.

Duplicate selection & Copy/Cut commands work differently based on the selection. I'd argue that a separate command is more confusing and annoying, but I guess both use cases might be useful, so idk.

@alexdima alexdima modified the milestones: April 2021, May 2021 Apr 30, 2021
@alexdima alexdima modified the milestones: May 2021, June 2021 Jun 7, 2021
@bpasero bpasero modified the milestones: June 2021, July 2021 Jul 9, 2021
@bpasero bpasero modified the milestones: July 2021, August 2021 Aug 9, 2021
@alexdima alexdima modified the milestones: August 2021, September 2021 Aug 26, 2021
@alexdima alexdima modified the milestones: September 2021, October 2021 Sep 29, 2021
@alexdima alexdima added the editor-commands Editor text manipulation commands label Oct 27, 2021
@alexdima
Copy link
Member

Thank you!

@alexdima alexdima merged commit 9264622 into microsoft:main Oct 27, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-commands Editor text manipulation commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants