Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Feature: New command - Delete Duplicate Lines#119480

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
alexdima merged 2 commits intomicrosoft:mainfromcodeclown:delete-duplicates
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 commentedMar 22, 2021
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@NotWearingPants
Copy link
Contributor

NotWearingPants commentedMar 23, 2021
edited
Loading

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
ContributorAuthor

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.

@alexdimaalexdima added this to theApril 2021 milestoneMar 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.

@alexdimaalexdima added the editor-commandsEditor text manipulation commands labelOct 27, 2021
@alexdima
Copy link
Member

Thank you!

@alexdimaalexdima merged commit9264622 intomicrosoft:mainOct 27, 2021
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsDec 11, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers
No reviews
Assignees

@alexdimaalexdima

Labels
editor-commandsEditor text manipulation commands
Projects
None yet
Milestone
October 2021
Development

Successfully merging this pull request may close these issues.

4 participants
@codeclown@NotWearingPants@alexdima@bpasero

[8]ページ先頭

©2009-2025 Movatter.jp