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

readline: add features yank and yank pop #41301

Merged
merged 1 commit into from
Jan 22, 2022
Merged

readline: add features yank and yank pop #41301

merged 1 commit into from
Jan 22, 2022

Conversation

rayw000
Copy link
Contributor

@rayw000 rayw000 commented Dec 23, 2021

  1. New features yank and yank pop
  2. Unit tests for the new features
  3. Document the new features

Fixes: #41252

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. labels Dec 23, 2021
@jasnell
Copy link
Member

jasnell commented Dec 23, 2021

+1 to adding this. Needs tests and doc updates also tho

@jasnell jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Dec 23, 2021
@rayw000
Copy link
Contributor Author

rayw000 commented Dec 23, 2021

+1 to adding this. Needs tests and doc updates also tho

No problem. I'll update tomorrow.

@rayw000 rayw000 changed the title repl: add feature yank and yank pop readline: add feature yank and yank pop Dec 24, 2021
@rayw000 rayw000 requested a review from jasnell December 24, 2021 04:25
@rayw000 rayw000 changed the title readline: add feature yank and yank pop readline: add features yank and yank pop Dec 24, 2021
doc/api/readline.md Outdated Show resolved Hide resolved
@rayw000
Copy link
Contributor Author

rayw000 commented Jan 18, 2022

This PR and #41392 are conflicting. Once one of them is accepted, I'll update the other.

@Ayase-252 Ayase-252 added the blocked PRs that are blocked by other issues or PRs. label Jan 21, 2022
@Ayase-252
Copy link
Member

Blocked on #41392

@Ayase-252 Ayase-252 removed the blocked PRs that are blocked by other issues or PRs. label Jan 21, 2022
@Ayase-252
Copy link
Member

#41392 was merged. Unblocked

1. `Ctrl-Y` to yank previously deleted text
2. `Meta-Y` to do yank pop (cycle among deleted texts)
3. Use `getCursorPos().rows` to check if we have reached a new line,
instead of `getCursorPos().cols === 0`.
4. document and unittests.
@Ayase-252 Ayase-252 added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 21, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 21, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Ayase-252 Ayase-252 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 22, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 22, 2022
@nodejs-github-bot nodejs-github-bot merged commit 2f17004 into nodejs:master Jan 22, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 2f17004

@rayw000 rayw000 deleted the feature-yank branch January 22, 2022 10:57
BethGriggs pushed a commit that referenced this pull request Jan 25, 2022
1. `Ctrl-Y` to yank previously deleted text
2. `Meta-Y` to do yank pop (cycle among deleted texts)
3. Use `getCursorPos().rows` to check if we have reached a new line,
instead of `getCursorPos().cols === 0`.
4. document and unittests.

PR-URL: #41301
Fixes: #41252
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Qingyu Deng <[email protected]>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
1. `Ctrl-Y` to yank previously deleted text
2. `Meta-Y` to do yank pop (cycle among deleted texts)
3. Use `getCursorPos().rows` to check if we have reached a new line,
instead of `getCursorPos().cols === 0`.
4. document and unittests.

PR-URL: nodejs#41301
Fixes: nodejs#41252
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Qingyu Deng <[email protected]>
@danielleadams
Copy link
Contributor

@rayw000 do you mind backporting this to v16.x? When landing the PR, it breaks tests.

@rayw000
Copy link
Contributor Author

rayw000 commented Apr 11, 2022

@rayw000 do you mind backporting this to v16.x? When landing the PR, it breaks tests.

Hi @danielleadams

This PR relies on #37947, which is similar to #41392. It may not be appropriate to backport #37947 since it is a semver major change.

@danielleadams
Copy link
Contributor

danielleadams commented Apr 11, 2022

@rayw000 no problem - I'll mark as don't-land-to-v16.x. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only Ctrl-Y yank doesn't work in REPL
6 participants