You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2022/08/01 22:10:02 UTC
[GitHub] [daffodil-vscode] scholarsmate commented on a diff in pull request #245: omega-edit updates
scholarsmate commented on code in PR #245:
URL: https://github.com/apache/daffodil-vscode/pull/245#discussion_r934939390
##########
src/omega_edit/omega_edit.ts:
##########
@@ -127,11 +84,101 @@ export class OmegaEdit {
}
}
+ async prepareSearch(fileSize: number): Promise<string> {
+ let segmentData = await omegaEditSession.getSegment(
+ this.sessionId,
+ 0,
+ fileSize
+ )
+ if (!segmentData) return ''
+ let sessionData = Buffer.from(segmentData).toString()
+
+ return sessionData
+ }
+
+ // Helper function to find the next index of the search patter in the data
+ findIndex = async (
+ indexOccurence: number,
+ sessionData: string,
+ searchPattern: string
+ ) => sessionData.indexOf(searchPattern, indexOccurence + 1)
+
+ // This need to be re done to use omega-edit scala server once
+ // search works properly
+ async search(
+ fileSize: number,
+ searchPattern: string
+ ): Promise<[string, Array<number>]> {
+ var sessionData = await this.prepareSearch(fileSize)
+
+ // let searchResults = await omegaEditSession.searchSession(
+ // this.sessionId,
+ // Buffer.from(searchPattern),
+ // true,
+ // 0,
+ // 0,
+ // fileSize
+ // ) <-- Currently not implemented scala side
+
+ var searchResults: Array<number> = []
+ var indexOccurence = await this.findIndex(-1, sessionData, searchPattern)
+
+ while (indexOccurence >= 0) {
+ searchResults.push(indexOccurence)
+ indexOccurence = await this.findIndex(
+ indexOccurence,
+ sessionData,
+ searchPattern
+ )
+ }
+
+ return [searchPattern, searchResults]
+ }
+
+ async replace(
+ sessionId: string,
+ offset: number,
+ len: number,
+ data: string | undefined
+ ) {
+ if (!data) return
Review Comment:
If data were empty or undefined, would it simply be a delete? If so, I'd put this conditional between the del and the insert. If instead it should be an error, then throw an Error instead of a silent return.
##########
src/omega_edit/omega_edit.ts:
##########
@@ -127,11 +84,101 @@ export class OmegaEdit {
}
}
+ async prepareSearch(fileSize: number): Promise<string> {
+ let segmentData = await omegaEditSession.getSegment(
+ this.sessionId,
+ 0,
+ fileSize
+ )
+ if (!segmentData) return ''
+ let sessionData = Buffer.from(segmentData).toString()
+
+ return sessionData
+ }
+
+ // Helper function to find the next index of the search patter in the data
+ findIndex = async (
+ indexOccurence: number,
+ sessionData: string,
+ searchPattern: string
+ ) => sessionData.indexOf(searchPattern, indexOccurence + 1)
+
+ // This need to be re done to use omega-edit scala server once
+ // search works properly
+ async search(
+ fileSize: number,
+ searchPattern: string
+ ): Promise<[string, Array<number>]> {
+ var sessionData = await this.prepareSearch(fileSize)
+
+ // let searchResults = await omegaEditSession.searchSession(
+ // this.sessionId,
+ // Buffer.from(searchPattern),
+ // true,
+ // 0,
+ // 0,
+ // fileSize
+ // ) <-- Currently not implemented scala side
+
+ var searchResults: Array<number> = []
+ var indexOccurence = await this.findIndex(-1, sessionData, searchPattern)
+
+ while (indexOccurence >= 0) {
+ searchResults.push(indexOccurence)
+ indexOccurence = await this.findIndex(
+ indexOccurence,
+ sessionData,
+ searchPattern
+ )
+ }
+
+ return [searchPattern, searchResults]
+ }
+
+ async replace(
+ sessionId: string,
+ offset: number,
+ len: number,
+ data: string | undefined
Review Comment:
When would this be undefined?
##########
src/omega_edit/utils.ts:
##########
@@ -60,7 +59,8 @@ export function viewportSubscribe(
commandViewport: string,
commandHex: string | null
) {
- client.subscribeToViewportEvents(new ObjectId().setId(vp1)).on('data', () => {
+ var objId = new ObjectId().setId(vp1)
+ client.subscribeToViewportEvents(objId).on('data', () => {
Review Comment:
This isn't good (I know this has been here a long time). The viewport event has the data needed to populate the viewport. There is no reason to use getViewportData causing a needless round trip and opening up a race condition. To see how you can extract the data you want, please reference https://github.com/ctc-oss/omega-edit/blob/main/src/rpc/client/ts/tests/specs/client.spec.ts#L63. Note also that if the viewport is floating you can extract the new offset from the event as well.
##########
src/omega_edit/omega_edit.ts:
##########
@@ -127,11 +84,101 @@ export class OmegaEdit {
}
}
+ async prepareSearch(fileSize: number): Promise<string> {
+ let segmentData = await omegaEditSession.getSegment(
+ this.sessionId,
+ 0,
+ fileSize
+ )
+ if (!segmentData) return ''
+ let sessionData = Buffer.from(segmentData).toString()
+
+ return sessionData
+ }
+
+ // Helper function to find the next index of the search patter in the data
+ findIndex = async (
+ indexOccurence: number,
+ sessionData: string,
+ searchPattern: string
+ ) => sessionData.indexOf(searchPattern, indexOccurence + 1)
+
+ // This need to be re done to use omega-edit scala server once
+ // search works properly
+ async search(
+ fileSize: number,
+ searchPattern: string
+ ): Promise<[string, Array<number>]> {
+ var sessionData = await this.prepareSearch(fileSize)
+
+ // let searchResults = await omegaEditSession.searchSession(
+ // this.sessionId,
+ // Buffer.from(searchPattern),
+ // true,
+ // 0,
+ // 0,
+ // fileSize
+ // ) <-- Currently not implemented scala side
+
+ var searchResults: Array<number> = []
+ var indexOccurence = await this.findIndex(-1, sessionData, searchPattern)
+
+ while (indexOccurence >= 0) {
+ searchResults.push(indexOccurence)
+ indexOccurence = await this.findIndex(
+ indexOccurence,
+ sessionData,
+ searchPattern
+ )
+ }
+
+ return [searchPattern, searchResults]
+ }
+
+ async replace(
+ sessionId: string,
+ offset: number,
+ len: number,
+ data: string | undefined
+ ) {
+ if (!data) return
+ await omegaEditChange.del(sessionId, offset, len)
+ await omegaEditChange.insert(sessionId, offset, data)
+ }
+
+ async searchAndReplace(
+ fileSize: number,
+ searchPattern: string,
+ replaceText: string
+ ) {
+ var sessionData = await this.prepareSearch(fileSize)
Review Comment:
Is there a check to ensure the search pattern isn't larger than the file size?
##########
src/omega_edit/client.ts:
##########
@@ -97,25 +97,31 @@ export function activate(ctx: vscode.ExtensionContext) {
randomId().toString(),
Review Comment:
I recommend leaving the desired session IDs undefined (unless they are explicitly named) to allow the server to generate them instead. The C++ server will use a 36-character UUID and the Scala server will use an 8-character ID. The servers make sure there are no collisions in the IDs.
##########
src/omega_edit/interface.html:
##########
@@ -169,6 +167,36 @@
<br />
<div style="padding-top: 35%; width: max-content">
+ <div class="setting-div">
+ <p>
+ Search: <input style="margin-left: 10px" id="searchInput" />
Review Comment:
Would be cool if we could validate this input to make sure it doesn't exceed the computed file size.
##########
src/omega_edit/omega_edit.ts:
##########
@@ -127,11 +84,101 @@ export class OmegaEdit {
}
}
+ async prepareSearch(fileSize: number): Promise<string> {
+ let segmentData = await omegaEditSession.getSegment(
Review Comment:
This is not good. I'm hoping that we're grabbing all the data as an interim hack until we get the Scala search service working? Either way, this code needs a comment declaring its purpose.
##########
src/omega_edit/omega_edit.ts:
##########
@@ -127,11 +84,101 @@ export class OmegaEdit {
}
}
+ async prepareSearch(fileSize: number): Promise<string> {
+ let segmentData = await omegaEditSession.getSegment(
+ this.sessionId,
+ 0,
+ fileSize
+ )
+ if (!segmentData) return ''
+ let sessionData = Buffer.from(segmentData).toString()
+
+ return sessionData
+ }
+
+ // Helper function to find the next index of the search patter in the data
Review Comment:
patter => pattern
##########
src/omega_edit/client.ts:
##########
@@ -97,25 +97,31 @@ export function activate(ctx: vscode.ExtensionContext) {
randomId().toString(),
s,
0,
- 1000
+ 1000,
Review Comment:
Are we limited to 1000 lines?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org