-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add createDiffView wrapper
#5825
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
Conversation
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5825 +/- ##
==========================================
+ Coverage 87.73% 87.77% +0.03%
==========================================
Files 618 620 +2
Lines 46046 46096 +50
Branches 7549 7559 +10
==========================================
+ Hits 40400 40461 +61
+ Misses 5646 5635 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…and update tests
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
| type GutterKeyboardEvent = import("ace-code/src/keyboard/gutter_handler").GutterKeyboardEvent; | ||
| type HoverTooltip = import("ace-code/src/tooltip").HoverTooltip; | ||
| type Tooltip = import("ace-code/src/tooltip").Tooltip; | ||
| type PopupManager = import("ace-code/src/tooltip").PopupManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional to remove PopupManager from our types here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it was on purpose. PopupManager was never exposed as class in code .
src/ext/diff.js
Outdated
| @@ -0,0 +1,31 @@ | |||
| var InlineDiffView = require("./diff/inline_diff_view").InlineDiffView; | |||
| var DiffView = require("./diff/diff_view").DiffView; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should call this one the split diff view :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I make an alias for that purpose? Or just rename the class?
ace.d.ts
Outdated
| /** | ||
| * Interface representing a model for handling differences between two views or states. | ||
| */ | ||
| export interface DiffModel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to add these types into the main ace.d.ts file? Can it be enough to just have these in ace-ext?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we could, but in terms of reusing better to do it through some .d.ts file (and ace-ext.d.ts is auto-generated). The only way to do this with js codebase (which I know) would be making jsdoc with @typedef tag. Also previously all interfaces were centralized in ace.d.ts under Ace namespace, not sure if that approach changed. Just poke me, if you still want it to be @typedef as PromptOptions for example
# Conflicts: # ace-internal.d.ts # ace.d.ts
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
…update usage across modules; add diff ext package tests
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
…functionality; add destroy calls for `diffView`
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
Issue #, if available:
Description of changes:
Add
createDiffViewwrapper to improve API and allow users to use extension in ace-buildsBy 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:
ace.d.ts) and its references: