Select between offsets #470
Conversation
src/selectBetweenOffsets.ts
Outdated
| let toOffset: number | undefined; | ||
| let accepted = false; | ||
|
|
||
| startingInput.onDidChangeValue(value => { |
There was a problem hiding this comment.
It seems we duplicate a lot of logic between the starting and ending inputs, can it be pulled into a helper function?
There was a problem hiding this comment.
Yes, working on it
src/selectBetweenOffsets.ts
Outdated
| const decimalRe = /^[0-9]+$/i; | ||
|
|
||
| export const showSelectBetweenOffsets = (messaging: ExtensionHostMessageHandler): void => { | ||
| const startingInput = vscode.window.createInputBox(); |
There was a problem hiding this comment.
It'd be quite nice for the default value in the starting input to be the currently selected byte. The HexDocument has a selectionState that you can read from.
There was a problem hiding this comment.
I was looking for that but failed to find the selected byte. Thanks, I will change it with that
There was a problem hiding this comment.
I couldn't access the active HexDocument. So I added a static variable holding that reference. Is this okay or is there a better way to access it? @connor4312
There was a problem hiding this comment.
You can get it via the activeDocument on the HexEditorRegistry (created inside activate())
There was a problem hiding this comment.
Ohh, how did I not see that?? Fixing it now
There was a problem hiding this comment.
Okay, I reverted my change in the HexDocument and get the ActiveDocument from registry. Now it should be good.
|
@microsoft-github-policy-service agree |
src/selectBetweenOffsets.ts
Outdated
| function changeValueHelper(value: string) { | ||
| if (!value) { | ||
| inputOffset = undefined; | ||
| } else if (addressRe.test(value)) { | ||
| inputOffset = parseInt(value.slice(2), 16); | ||
| } else if (decimalRe.test(value)) { | ||
| inputOffset = parseInt(value, 10); | ||
| } else { | ||
| startingInput.validationMessage = "Offset must be provided as a decimal (12345) or hex (0x12345) address"; | ||
| return; | ||
| } | ||
|
|
||
| startingInput.validationMessage = ""; | ||
| if (inputOffset !== undefined) { | ||
| startingInput.validationMessage = ""; | ||
| messaging.sendEvent({ type: MessageType.GoToOffset, offset: inputOffset }); | ||
| } | ||
| } |
There was a problem hiding this comment.
This function always affects the startingInput, even though it's used for the endingInput as well
There was a problem hiding this comment.
My bad, thanks for noticing
src/selectBetweenOffsets.ts
Outdated
|
|
||
| startingInput.show(); | ||
|
|
||
| function hideHelper() { |
There was a problem hiding this comment.
you'll want to make sure to .dispose() each input after their used.
Reworking this function to use some inner function getOffset(options: { ... }): Promise<number> would probably be easier to deal with
There was a problem hiding this comment.
I thought onDidHide already disposes the InputBox. In goToOffset.ts file, there isn't a .dispose() for the input. And if I call .dispose() in hideHelper function, hideHelper gets called twice. Should I still call .dispose()?
There was a problem hiding this comment.
Yes, we should always call dispose. There are additional resources that get collected.
There was a problem hiding this comment.
I did what you suggested and I think it turned out great. Let me know if there are any problems.
Implements a simple command that enables user to select byte values between 2 offsets.
Useful for extracting embedded files.
closes #469