feat: add Java support for Functions debugging #3273
Conversation
| @@ -76,6 +76,9 @@ export class ForceFunctionStartExecutor extends SfdxCommandletExecutor<string> { | |||
| } | |||
|
|
|||
| public execute(response: ContinueResponse<string>) { | |||
| const runtimeDetection: RegExp = new RegExp( | |||
There was a problem hiding this comment.
nit: upper case and move out of the class / make static since this is a constant
There was a problem hiding this comment.
also in the variable name describe we are detecting debug type here
| @@ -143,7 +147,10 @@ export class ForceFunctionStartExecutor extends SfdxCommandletExecutor<string> { | |||
| ); | |||
|
|
|||
| execution.stdoutSubject.subscribe(data => { | |||
| if (data.toString().includes('Debugger running on port')) { | |||
| const matches = String(data).match(runtimeDetection); | |||
There was a problem hiding this comment.
Curious if this is the only option to find the function type? Any information about the language or the directory structure at the time of registering the function we could use instead?
I'm just trying to avoid the need to maintain a regex for any future changes in the output.
There was a problem hiding this comment.
That was my first approach, but I decided to change it. Since the build-pack is creating the environment seems like it is best suited to determine the runtime/debug settings.
There was a problem hiding this comment.
In that case, can we add an else condition for the line below where we check for matches and give a user warning that reads something like "Language could not be identified. 'Debug Invoke' may not work".
There was a problem hiding this comment.
I'll see if I can move that type of logic to the 'Debug Invoke' - it should not be visible/enabled if the language hasn't been determined, or at least produce the warning you mentioned.
There was a problem hiding this comment.
Also looks like there is enough business logic now to add a separate section of tests in forceFunctionStart.test.ts
| const matches = String(data).match(runtimeDetection); | ||
| if (matches && matches.length > 1) { | ||
| FunctionService.instance.updateFunction(functionDirPath, matches[1]); | ||
| } else if (data.toString().includes('Debugger running on port')) { |
There was a problem hiding this comment.
why is here an else condition? Don't we want to close the notification irrespective of a language match or not.
There was a problem hiding this comment.
The language match occurs fairly early, whereas the "running" message is one of the last messages.
There was a problem hiding this comment.
Synced offline and now understand data is one line of stdout at a time. So, this is fine.
What does this PR do?
What issues does this PR fix or reference?
@W-9330349@