-
Notifications
You must be signed in to change notification settings - Fork 19
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
Fix for storing table extension field and enumextension values on base table, Make Range and Assignment Explorer work (mostly) with simple pools and other smaller stuff #78
base: master
Are you sure you want to change the base?
Conversation
…without description
….50000) for enum values
…thout a valid number)
…tly it would show all object ids of pool apps that are not in the workspace
…onsumption in the change doesn't reflect in the pool view
…etChildrenOfLogicalRangesGroupNode, extract getDescriptionOfRange, add DEFAULT_RANGE_DESCRIPTION
Cache on WorkspaceManager level. Each app loads it's dependencies. Dependency Loading is done by loading the SymbolReference.json from each .app file Whenever the dependencies might be used (e.g. when searching for the base table/enum id), the dependencies are checked if they're up to date. Mostly the cache comes into play then.
…ject` is disabled Add logging in case of failures add xml documentation to some procedures
…use usings?) and fix wrong parser extends-info
… file were not supported. Using Map<string> instead
… it was way too often not respected, so I guess vjeko did not want to use it.
…ge explorer view for pools
…nged (maybe now part of pool or vice-versa)
…w also supports pools and it has been made less error-prone by turning off some file watchers while switching branches.
f6673d0
to
1d59d6c
Compare
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.
Let's try to get this merged. I am really struggling finding time, but this seems ready. There are some mostly cosmetic fixes to this.
I didn't test everything you did, but from what I see, it seems that everyone will have to auto-sync everything again, otherwise their fields/enum values assignments won't work. Correct?
vscode-extension/src/features/treeView/assignmentExplorer/nodes/AssignmentExplorerRootNode.ts
Outdated
Show resolved
Hide resolved
vscode-extension/src/features/treeView/assignmentExplorer/nodes/CollisionNode.ts
Outdated
Show resolved
Hide resolved
...tension/src/features/treeView/assignmentExplorer/nodes/fieldLostNodes/LostFieldObjectNode.ts
Outdated
Show resolved
Hide resolved
...tension/src/features/treeView/assignmentExplorer/nodes/fieldLostNodes/LostFieldObjectNode.ts
Outdated
Show resolved
Hide resolved
vscode-extension/src/features/treeView/rangeExplorer/RangeExplorerView.ts
Outdated
Show resolved
Hide resolved
…preview.background`
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.
Hey Vjeko, I fixed your comments so far. I left two comments on "unresolved" simply to have a last look into it, but I would say they can be resolved as well.
I would like to have a call tomorrow or sometime next week with you to talk about it, show what's new now and what to have in mind. E.g. not each company using this app has to resync everything. Only if they activate a setting which fixes the problem that field ids are currently stored incorrectly.
@@ -58,29 +60,30 @@ export class Diagnostics implements Disposable { | |||
//#endregion | |||
|
|||
private readonly _diagnostics: DiagnosticCollection; | |||
private _documents = new WeakMap<Uri, PropertyBag<Diagnostic[]>>(); | |||
private _schedulers = new WeakMap<Uri, NodeJS.Timeout>(); | |||
private _documents = new Map<string, PropertyBag<Diagnostic[]>>(); |
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.
If I remember correctly then _documents.get(uri)
and _documents.delete(uri)
wasn't working, so duplicated entries were added. Don't know if I can reproduce it again to make sure that turning it back won't result in an issue. Would rather prefer to leave it like this if that's fine.
vscode-extension/src/features/treeView/assignmentExplorer/nodes/AssignmentExplorerRootNode.ts
Outdated
Show resolved
Hide resolved
vscode-extension/src/features/treeView/assignmentExplorer/nodes/CollisionNode.ts
Outdated
Show resolved
Hide resolved
...tension/src/features/treeView/assignmentExplorer/nodes/fieldLostNodes/LostFieldObjectNode.ts
Outdated
Show resolved
Hide resolved
vscode-extension/src/features/treeView/rangeExplorer/RangeExplorerView.ts
Outdated
Show resolved
Hide resolved
...tension/src/features/treeView/assignmentExplorer/nodes/fieldLostNodes/LostFieldObjectNode.ts
Outdated
Show resolved
Hide resolved
…n uncompilable state already, then nothing happened.
… available anymore, popped up even if it could fall back to the idRanges of the app.json
Added
fieldAndValueIdsStayInsideObjectRange
which makes sure that if you've numbers per object range, e.g. per tableextension, that the field id's stay inside that range as well to avoid conflicts with others.storeExtensionValuesOrIdsOnBaseObject
: With this, reserved object and field numbers are stored on base table/enum level. Currently it's done on tableextension or enumextension level which might lead to conflicts if you've multiple extensions on that same base table/enum. We strongly recommend to activate this setting, but that requires that you update your consumption afterwards e.g. using theNinja: Automatically Synchronize Object IDs for Entire Workspace
command as basically all tableextension field and enumextension value ids are then not considered to be consumed anymore. By activating this setting, the dependencies are loaded using the SymbolReference.json to find out in a fast way the extended object name -> object id relation. The performance impact should not be noticable as it's using as much caching as possible and loads the dependencies already on vscode startup.Range Explorer
andAssignment Explorer
View accessible for App Pools as well. TheLost ids
part of theAssignment Explorer
is not yet working for pools.Assignment Explorer
.Changed
fieldAndValueIdsStayInsideObjectRange
setting.Fixed
Range Explorer
View could not deal with multiple object ranges without a description.