Edit session operation #5657

marinsokol5 · 2024-10-18T08:38:23Z

What

Adding a concept of an "operation" to EditSession, the same one that we right now have in the Editor
When EditSession is attached to an Editor, the Editor listens to the events from EditSession and EditSession is the true owner of the event.
When EditSession is on it's own, it can only get programmatic changes, in which case it relies on programmatic handling of the operation lifecycle, where we still handle gracefully when that doesn't happen as expected.

Why

When integrating Ace in a multiple files experiences, such as IDEs or just an editor with a tab bar, you work with multiple EditSessions and you swap them out within the Editor depending on which one is currently active/focussed.
It's very useful for those integrators to freely do changes to EditSession, even when one is not mounted to Editor, and listen to changes through beforeEndOperation to avoid intermediary states.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Pull Request Checklist:

No backwards incompatible changes were made to Ace's public interface which is defined through the main typings file (ace.d.ts) and its references:

github-actions · 2024-10-18T08:38:35Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

github-actions · 2024-10-18T08:41:31Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

github-actions · 2024-10-18T08:44:33Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

codecov · 2024-10-18T08:45:31Z

Codecov Report

Attention: Patch coverage is 99.32886% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.00%. Comparing base (560f5d3) to head (a44f0d1).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/edit_session.js	98.07%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5657      +/-   ##
==========================================
+ Coverage   86.97%   87.00%   +0.03%     
==========================================
  Files         594      594              
  Lines       43395    43516     +121     
  Branches     7170     7174       +4     
==========================================
+ Hits        37742    37862     +120     
- Misses       5653     5654       +1

Flag	Coverage Δ
unittests	`87.00% <99.32%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-10-18T08:48:10Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

github-actions · 2024-10-18T12:06:02Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

github-actions · 2024-10-18T13:37:20Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

github-actions · 2024-10-18T14:03:25Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

github-actions · 2024-10-18T14:28:20Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

akoreman · 2024-10-18T15:25:53Z

Could you add a brief description to the PR description why we want to make this change and what type of features we unblock by this?

nightwing · 2024-10-20T11:22:36Z

src/editor.js

+    }
+
+    onStartOperation(commandEvent) {
+        // scrollTop is kept inside of session.curOp only for backwards compatibility reasons


Just because the bottom line this.curOp = this.session.curOp = ... was inserting it.
So someone could had have integration code doing something like session.curOp.scrollTop. I fully agree though that conceptually this shouldn't be there...

Regarding backwards compatibility, are you okay with just treating this as an implementation detail on our end, since we don't really officially document it in

ace/ace.d.ts

Line 507 in 37c5f76

export interface EditSession extends EventEmitter, OptionsProvider, Folding {

or in https://ajaxorg.github.io/ace-api-docs/index.html?

nightwing · 2024-10-20T11:23:36Z

src/editor.js

@@ -347,6 +326,11 @@ class Editor {

            this.$onSelectionChange = this.onSelectionChange.bind(this);
            this.selection.on("changeSelection", this.$onSelectionChange);
+
+            this.$onStartOperation = this.onStartOperation.bind(this);


This is better to do in constructor to not recreate the bound function every time

Isn't this exactly what we want, since this listener belongs to a session and should be created and cleaned up according to the session lifecycle?

github-actions · 2024-10-21T11:23:47Z

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

sokomari added 2 commits October 17, 2024 18:15

feat(edit-session): adding operation handling

e5b9b24

decorative updates

e3e277a

marinsokol5 requested a review from nightwing October 18, 2024 08:38

linting issues

33b264f

typing issues

756738d

removing console.log

19d1e11

editor listening to edit-session operation events

649f83c

fixing build

f23f456

typescript issue

9e2a30c

checking editor operationEnd listener too

f2376b1

nightwing reviewed Oct 20, 2024

View reviewed changes

removing comment

a44f0d1

nightwing approved these changes Oct 21, 2024

View reviewed changes

marinsokol5 merged commit aba0575 into master Oct 21, 2024
4 checks passed

marinsokol5 deleted the edit-session-operation branch October 21, 2024 12:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Edit session operation #5657

Edit session operation #5657

marinsokol5 commented Oct 18, 2024 •

edited

Loading

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

codecov bot commented Oct 18, 2024 •

edited

Loading

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

akoreman commented Oct 18, 2024

nightwing Oct 20, 2024

marinsokol5 Oct 21, 2024

nightwing Oct 20, 2024

marinsokol5 Oct 21, 2024

github-actions bot commented Oct 21, 2024

Edit session operation #5657

Edit session operation #5657

Conversation

marinsokol5 commented Oct 18, 2024 • edited Loading

What

Why

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

codecov bot commented Oct 18, 2024 • edited Loading

Codecov Report

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

akoreman commented Oct 18, 2024

nightwing Oct 20, 2024

Choose a reason for hiding this comment

marinsokol5 Oct 21, 2024

Choose a reason for hiding this comment

nightwing Oct 20, 2024

Choose a reason for hiding this comment

marinsokol5 Oct 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 21, 2024

marinsokol5 commented Oct 18, 2024 •

edited

Loading

codecov bot commented Oct 18, 2024 •

edited

Loading