Emacs text registers #1643

justinhopkins · 2023-08-23T00:36:45Z

Greetings whitphx!

First of all, thank you for maintaining awesome-emacs-mcx! It is unbelievably excellent for emacs nerds like myself.

I've been experimenting with implementing Emacs text registers. "Text registers" are effectively multiple clipboards that you can identify with a single keystroke. Full-blown emacs registers can do a lot, but so far I'm just trying to implement two commands - "register save" and "register insert".

This implementation seems to play nicely with the C-x r commands that work with rectangles.

There are some extensive changes, so I expect you may object to some design decisions. I'm happy to make changes based on your feedback.

Hope to hear from you

Reference:
https://www.gnu.org/software/emacs/manual/html_node/emacs/Text-Registers.html

^ From the documentation:

register save:
C-x r s r
Copy region into register r (copy-to-register).

register insert:
C-x r i r
Insert text from register r (insert-register).

…that play nicely with existing Rect-related commands

whitphx

Thank you very much for your contribution!
Let me point out some things as comments.

Also, CI workflows have not been dispatched maybe because of misconfiguration. I updated the CI definition file on the main branch, so please update this branch with it and see if the CI is triggered.

whitphx · 2023-08-27T13:56:17Z

src/emulator.ts

@@ -36,8 +38,25 @@ export interface IEmacsController {
  registerDisposable: (disposable: vscode.Disposable) => void;
 }

+export class TextRegister {


I think you can just use a Map class instead of wrapping it with TextRegister class.

Agreed. I've updated it to just use a Map<string, string>

whitphx · 2023-08-27T13:59:12Z

src/emulator.ts

+      i++;
+    }
+    const msg = "Running RegisterSaveCommand to " + arg + " with this selection: " + combinedtext;
+    vscode.window.showInformationMessage(msg);


Please consider using MessageManager.showMessage which manages the lifespan of each message.

like this:

vscode-emacs-mcx/src/emulator.ts

Line 441 in 6f89b5b

MessageManager.showMessage("Quit");

Roger that; changed it

whitphx · 2023-08-27T13:59:54Z

src/emulator.ts

@@ -93,6 +112,7 @@ export class EmacsEmulator implements IEmacsController, vscode.Disposable {
    this.textEditor = textEditor;
    this._nativeSelections = this.rectMode ? [] : textEditor.selections; // TODO: `[]` is workaround.

+    this.textRegister = new TextRegister();


One EmacsEmulator instance is bound to one document, so looks like this text register is not shared among different documents.
Instead, I think it should be changed so that a single global text register object is initialized in the activate() function in extension.ts and it is passed to each EmacsEmulator instance like killRing.

whitphx · 2023-08-27T14:06:12Z

src/emulator.ts

@@ -541,4 +564,61 @@ export class EmacsEmulator implements IEmacsController, vscode.Disposable {
      }
    });
  }
+
+  public saveRegister(arg: string): void {


It would be great if you write tests for the new commands.
Just leave it if it's hard, as I will make some after merging the PR.

Understood. Please have a look at how I've modified EmulatorMap and EmacsEmulator to have a textRegister Map(). If this approach looks good, I'll start writing unit tests

whitphx · 2023-08-27T14:25:12Z

src/emulator.ts

+    let i = 0;
+    let combinedtext = "";
+    while (i < selections.length) {
+      combinedtext = combinedtext + this.textEditor.document.getText(selections[i]);


While it's not needed in this PR, the copy/paste functions of the kill/yank commands should be shared with these methods. They are compatible with multi-cursors.

A good point. Still researching

- Changed vscode.window.showInformationMessage() to MessageManager.showMessage() - Made RegisterSaveCommand de-select the selection after saving the selection

…e document windows (multiple EmacsEmulator objects)

Merging for update

justinhopkins · 2023-09-04T19:30:44Z

Thank you very much for your contribution! Let me point out some things as comments.

Also, CI workflows have not been dispatched maybe because of misconfiguration. I updated the CI definition file on the main branch, so please update this branch with it and see if the CI is triggered.

Re-merged with your changes on 'main' for changes from the .github/workflows/main.yaml.

Do you need me to do anything else for this PR? Would be happy to make changes if I've missed anything.

I'm also kind of waiting for your thoughts about how I've added textRegister as an object passed into the EmacsEmulator. If it looks alright, I'll go ahead and write some test cases for it.

whitphx · 2023-09-10T11:09:15Z

@justinhopkins Thank you so much for your contributions and sorry for my late reply.
The current implementation looks nice and I think it's ready to write the tests.
Should I leave this PR open until you write the tests, or merge it first?

whitphx

I tested this locally and found a few things.
Let me suggest them ;)

whitphx · 2023-09-10T11:09:49Z

src/commands/registers.ts

+
+  private acceptingRegisterSaveCommand = false;
+
+  public startRegisterSaveCommand(): void {


Can you make this private?

Sorry this was originally my bad.
Fixed the original code you referred to as the basis: #1680

src/commands/registers.ts

src/emulator.ts

- Made startRegisterSaveCommand() private - Made textEditor.edit((editBuilder) ...) use await in saveRegister() method

justinhopkins · 2023-09-10T18:12:14Z

@justinhopkins Thank you so much for your contributions and sorry for my late reply. The current implementation looks nice and I think it's ready to write the tests. Should I leave this PR open until you write the tests, or merge it first?

No worries! Just made changes based on your feedback. Please merge it first, and I'll follow-up with unit tests!
Thanks!

whitphx · 2023-09-12T07:25:00Z

OK, thanks!

whitphx · 2023-09-15T05:24:40Z

Released in v0.50.0 👍

justinhopkins added 2 commits August 22, 2023 19:26

New methods to create "register save" and "register insert" commands …

28a579e

…that play nicely with existing Rect-related commands

New methods to create "register save" and "register insert" commands …

24e1a47

…that play nicely with existing Rect-related commands

whitphx reviewed Aug 27, 2023

View reviewed changes

justinhopkins added 5 commits September 1, 2023 15:09

- Changed TextRegister class to be a simple Map<string, string>

24bf6ec

- Changed vscode.window.showInformationMessage() to MessageManager.showMessage() - Made RegisterSaveCommand de-select the selection after saving the selection

Removed commented region

6fef96c

Per feedback - refactoring text registers to be shared across multipl…

52be872

…e document windows (multiple EmacsEmulator objects)

Merge pull request #1 from whitphx/main

6b723ff

Merging for update

Merge branch 'main' into Emacs_TextRegisters

022b121

whitphx reviewed Sep 10, 2023

View reviewed changes

- Removed extraneous not-in-vanilla-Emacs messages

580972c

- Made startRegisterSaveCommand() private - Made textEditor.edit((editBuilder) ...) use await in saveRegister() method

whitphx merged commit 42fda5b into whitphx:main Sep 12, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emacs text registers #1643

Emacs text registers #1643

justinhopkins commented Aug 23, 2023

whitphx left a comment

whitphx Aug 27, 2023

justinhopkins Sep 1, 2023

whitphx Aug 27, 2023

justinhopkins Sep 1, 2023

whitphx Aug 27, 2023

whitphx Aug 27, 2023

justinhopkins Sep 1, 2023

whitphx Aug 27, 2023

justinhopkins Sep 1, 2023

justinhopkins commented Sep 4, 2023

whitphx commented Sep 10, 2023

whitphx left a comment •

edited

Loading

whitphx Sep 10, 2023

whitphx Sep 24, 2023

justinhopkins commented Sep 10, 2023

whitphx commented Sep 12, 2023

whitphx commented Sep 15, 2023


		private acceptingRegisterSaveCommand = false;

		public startRegisterSaveCommand(): void {

Emacs text registers #1643

Emacs text registers #1643

Conversation

justinhopkins commented Aug 23, 2023

whitphx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinhopkins commented Sep 4, 2023

whitphx commented Sep 10, 2023

whitphx left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinhopkins commented Sep 10, 2023

whitphx commented Sep 12, 2023

whitphx commented Sep 15, 2023

whitphx left a comment •

edited

Loading