Skip to content

feat: Implement editor.convertLineEndingOnCopy and editor:paste-without-reindenting #1229

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

icecream17
Copy link
Contributor

Currently, when copying, line endings are converted to the system EOL.

This is useful for copying and pasting into another program, but it has the drawback of copying and Ctrl+Shift+V on a LF file on Windows causing inconsistent line endings.

Two things are added for this:

  • editor.convertLineEndingOnCopy: The "off" option allows to not convert on copy
  • editor:paste-without-reindenting: This new command is like editor:paste-without-reformatting but just for indenting. So line endings are still converted and trailing white space is still removed

Both of these are potentially useful, but to avoid feature creep ideally others could weigh in.

(note the first commit's message for reviewing)
There aren't any tests for the command `editor:paste-without-reformatting` (just for all the options passed to the `paste` method itself), so I didn't add any for the new command `editor:paste-without-reindenting`

Also some documentation from my investigation of PaneElement a year ago.

The single-line PaneElement description may seem useless, but I first thought it was an entry of a pane (those are pane items).

image

Links

Discord conversation: https://canary.discord.com/channels/992103415163396136/992103415163396139/1342613487880376382

Issue and pr for original functionality: atom/atom#8365 and atom/atom#19016

Release notes

  • Add editor.convertLineEndingOnCopy setting.
  • Add editor:paste-without-reindenting command.

icecream17 and others added 2 commits February 22, 2025 00:34
…ut-reindenting

There aren't any tests for the command `editor:paste-without-reformatting` (just for all the options passed to the `paste` method itself), so I didn't add any for the new command `editor:paste-without-reindenting`

Also some documentation from my investigation of PaneElement a year ago.

The single-line PaneElement description may seem useless, but I first thought it was an entry of a pane (those are pane items).
when the outer "describe" is run, the config is set multiple times. Then the tests are run: https://jestjs.io/docs/setup-teardown#order-of-execution

This results in unexpected behaviour
Copy link
Contributor

@savetheclocktower savetheclocktower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reactionary gut feeling is that the ideal fix for your situation would've been the definition of a custom “Paste without reindenting” command in your init.js, and that we don't actually need to touch the core codebase at all.

If I were to argue against myself, I'd point out that most people wouldn't write such a command until after they'd gone through the pain of realizing they needed it, and we might as well save them the time. Considering how simple it is to define editor:paste-without-reindenting, we might as well make it come up in a command palette, especially since it's not bound to anything by default. I'd probably remap Cmd-Shift-V to that command myself, since I didn't even realize editor:paste-without-reformatting skipped normalization of line endings (which is pretty ridiculous if I think about it).

The ongoing cost of supporting a new setting is far greater than that of adding a single command, so I would be warmly in favor of a version of this PR that added nothing but the command.

I could be convinced that theeditor.convertLineEndingOnCopy setting is worth adding if someone can make an affirmative case for it, but I think the behavior here (normalizing line endings on copy to agree with those of the OS and make the text easily paste-able into any other application) is self-evidently correct, and that the real problem was that editor:paste-without-reformatting didn't normalize line endings. (Despite the command name, I'm betting nobody expects “without reformatting” to mean “possibly introducing the wrong kind of line ending.”)

If others feel it should be added, I'm happy to be outvoted. But at the very least it should be a checkbox — convert or don't convert — because I'm skeptical that a user might want to opt into normalization and yet pick an arbitrary line ending style that doesn't match that of their own system. If a Windows Pulsar user went hardcore \n-only, they'd still have to accept normalization on paste in order to be able to copy text from Stack Overflow, their favorite AI coding buddy, etc., and paste it into Pulsar.

So that's why I'm 👎 on this PR as it is, but feel free to wait until someone else weighs in.

@icecream17
Copy link
Contributor Author

icecream17 commented Feb 26, 2025

To make the options more pollable (3x2:)

  • close the pr (the command can be implemented in the init script)
  • just add the command (paste-without-indenting)
  • change paste-without-formatting to convert newlines

  • the convertLineEndingOnCopy setting isn't necessary
  • others have a use case for the convertLineEndingOnCopy setting (though probably the LF and CRLF options are not needed anyways)

I'll admit, I just added the LF and CRLF options from atom/atom#8365 (comment) without really thinking about it. I'll wait for further conversation before choosing one of the above actions, but I'm thinking the third option in the first list and the first option in the second list is most likely.

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 this pull request may close these issues.

2 participants