feat: support cwd and env variable using sun jdi by thunderz99 · Pull Request #45 · fwcd/kotlin-debug-adapter

thunderz99 · 2020-07-02T07:44:18Z

An enhance for #39

Done

cwd
- Add "cwd" for LaunchConfiguration
  - default to ${projectRoot} if null or blank
- Add a custom KDACommandLineLauncher which implements LaunchingConnector
  - The implementation is in original code written in Kotlin
- Add unit test for cwd
env variables
- Add "envs" for LaunchConfiguration
- Change "envs" config from List<String> to Map<String, String> style for more friendly user experience
- Add unit test for envs
support for jdk1.8
support for jdk11
- SunCommandLineLauncher cannot be extended under jdk11, so KDACommandLineLauncher implements
  LaunchingConnector directly
- SocketTransportService cannot be constructed under jdk11, so it is newed by Class.forName

Linked pullreq

Add cwd / envs support in vscode-kotlin
feat: add cwd and env variables support vscode-kotlin#42

bkleiner · 2021-04-14T12:06:05Z

any update on this? what needs to be done to get this merged?

fwcd

Sorry for the long delay here, thanks for looking into this! I have left a few comments (mostly concerning style) below, so we can get this moving.

fwcd · 2022-06-09T17:24:59Z

 	// The Java Debug Interface classes (com.sun.jdi.*)
 	implementation files("${System.properties['java.home']}/../lib/tools.jar")
+	// For CommandLineUtils.translateCommandLine
+	implementation 'org.codehaus.plexus:plexus-utils:3.3.0'


If possible, I would like to avoid introducing a new dependency here. For splitting the command line args, I think just splitting by spaces would be fine for now.

fwcd · 2022-06-09T17:27:57Z

+/**
+ * A custom LaunchingConnector that supports cwd and env variables
+ */
+open class KDACommandLineLauncher : LaunchingConnector {


Is there a specific reason to make this open, e.g. do we need to do so for interop with Java/JDI? If not, I would suggest removing the modifier.

fwcd · 2022-06-09T17:28:59Z

+        defaultArguments[ARG_HOME] = StringArgument(ARG_HOME, description = "Java home", value = System.getProperty("java.home"))
+
+        defaultArguments[ARG_OPTIONS] = StringArgument(ARG_OPTIONS, description = "Jvm arguments")
+
+        defaultArguments[ARG_MAIN] = StringArgument(ARG_MAIN, description = "Main class name and parameters", mustSpecify = true)
+
+        defaultArguments[ARG_SUSPEND] = StringArgument(ARG_SUSPEND, description = "Whether launch the debugee in suspend mode", value = "true")
+
+        defaultArguments[ARG_QUOTE] = StringArgument(ARG_QUOTE, description = "Quote char", value = "\"")
+
+        defaultArguments[ARG_VM_EXEC] = StringArgument(ARG_VM_EXEC, description = "The java exec", value = "java")
+
+        defaultArguments[ARG_CWD] = StringArgument(ARG_CWD, description = "Current working directory")
+
+        defaultArguments[ARG_ENVS] = StringArgument(ARG_ENVS, description = "Environment variables")


Style nit: Please remove the blank lines inbetween.

Suggested change

defaultArguments[ARG_HOME] = StringArgument(ARG_HOME, description = "Java home", value = System.getProperty("java.home"))

defaultArguments[ARG_OPTIONS] = StringArgument(ARG_OPTIONS, description = "Jvm arguments")

defaultArguments[ARG_MAIN] = StringArgument(ARG_MAIN, description = "Main class name and parameters", mustSpecify = true)

defaultArguments[ARG_SUSPEND] = StringArgument(ARG_SUSPEND, description = "Whether launch the debugee in suspend mode", value = "true")

defaultArguments[ARG_QUOTE] = StringArgument(ARG_QUOTE, description = "Quote char", value = "\"")

defaultArguments[ARG_VM_EXEC] = StringArgument(ARG_VM_EXEC, description = "The java exec", value = "java")

defaultArguments[ARG_CWD] = StringArgument(ARG_CWD, description = "Current working directory")

defaultArguments[ARG_ENVS] = StringArgument(ARG_ENVS, description = "Environment variables")

defaultArguments[ARG_HOME] = StringArgument(ARG_HOME, description = "Java home", value = System.getProperty("java.home"))

defaultArguments[ARG_OPTIONS] = StringArgument(ARG_OPTIONS, description = "Jvm arguments")

defaultArguments[ARG_MAIN] = StringArgument(ARG_MAIN, description = "Main class name and parameters", mustSpecify = true)

defaultArguments[ARG_SUSPEND] = StringArgument(ARG_SUSPEND, description = "Whether launch the debugee in suspend mode", value = "true")

defaultArguments[ARG_QUOTE] = StringArgument(ARG_QUOTE, description = "Quote char", value = "\"")

defaultArguments[ARG_VM_EXEC] = StringArgument(ARG_VM_EXEC, description = "The java exec", value = "java")

defaultArguments[ARG_CWD] = StringArgument(ARG_CWD, description = "Current working directory")

defaultArguments[ARG_ENVS] = StringArgument(ARG_ENVS, description = "Environment variables")

fwcd · 2022-06-09T17:30:20Z

+            command.append(wrapWhitespace(exe))
+
+            command.append(" $options")
+
+            //debug options
+            command.append(" -agentlib:jdwp=transport=${transport.name()},address=$address,server=n,suspend=${if (suspend) 'y' else 'n'}")
+
+            command.append(" $main")


Same here, I think it looks a bit nicer with less whitespace.

Suggested change

command.append(wrapWhitespace(exe))

command.append(" $options")

//debug options

command.append(" -agentlib:jdwp=transport=${transport.name()},address=$address,server=n,suspend=${if (suspend) 'y' else 'n'}")

command.append(" $main")

command.append(wrapWhitespace(exe))

command.append(" $options")

// debug options

command.append(" -agentlib:jdwp=transport=${transport.name()},address=$address,server=n,suspend=${if (suspend) 'y' else 'n'}")

command.append(" $main")

fwcd · 2022-06-09T17:31:34Z

+    fun launch(commandArray: Array<String>,
+                        listenKey: TransportService.ListenKey,
+                        ts: TransportService, cwd: String, envs: Array<String>? = null): VirtualMachine {


I think this would read better if we moved the parameters to individual lines:

Suggested change

fun launch(commandArray: Array<String>,

listenKey: TransportService.ListenKey,

ts: TransportService, cwd: String, envs: Array<String>? = null): VirtualMachine {

fun launch(

commandArray: Array<String>,

listenKey: TransportService.ListenKey,

ts: TransportService,

cwd: String,

envs: Array<String>? = null

): VirtualMachine {

fwcd · 2022-06-09T17:32:37Z

+        return Bootstrap.virtualMachineManager().createVirtualMachine(connection,
+                process)


Since we already have a bunch of longer lines, I think not breaking would be fine here.

Suggested change

return Bootstrap.virtualMachineManager().createVirtualMachine(connection,

process)

return Bootstrap.virtualMachineManager().createVirtualMachine(connection, process)

fwcd · 2022-06-09T17:33:48Z

+    protected fun launchAndConnect(commandArray: Array<String>, listenKey: TransportService.ListenKey,
+                         ts: TransportService, cwd: String = "", envs: Array<String>? = null): Pair<Connection, Process>{


Suggested change

protected fun launchAndConnect(commandArray: Array<String>, listenKey: TransportService.ListenKey,

ts: TransportService, cwd: String = "", envs: Array<String>? = null): Pair<Connection, Process>{

protected fun launchAndConnect(

commandArray: Array<String>,

listenKey: TransportService.ListenKey,

ts: TransportService,

cwd: String = "",

envs: Array<String>? = null

): Pair<Connection, Process> {

fwcd · 2022-06-09T17:36:04Z

+class StringArgument constructor(private val name: String, private val description: String = "", private val label: String = name,
+                                     private var value:String = "", private val mustSpecify: Boolean = false) : Connector.Argument {


Could we omit the constructor here, since it is the primary constructor?

Suggested change

class StringArgument constructor(private val name: String, private val description: String = "", private val label: String = name,

private var value:String = "", private val mustSpecify: Boolean = false) : Connector.Argument {

class StringArgument(

private val name: String,

private val description: String = "",

private val label: String = name,

private var value:String = "",

private val mustSpecify: Boolean = false

) : Connector.Argument {

fwcd · 2022-06-09T17:38:10Z

+    }
+
+
+}


It would be great if you could make sure that all edited files have trailing newlines (by Unix convention). Most editors should be configurable to do so, e.g. in VSCode you can set

"files.insertFinalNewline": true

in your settings.

thunderz99 · 2022-07-02T06:39:53Z

@fwcd Thank you for the advices, I will update this PR

aharoldk · 2025-03-09T05:08:00Z

@thunderz99 please update the MR.

thunderz99 marked this pull request as draft July 2, 2020 07:44

feat: support cwd anv env variables using sun jdi

15203b1

thunderz99 force-pushed the feat/support-cwd-and-env-variable-using-sun-jdi branch from 34a9e2e to 15203b1 Compare July 2, 2020 07:57

add support to jdk11

64821ae

thunderz99 marked this pull request as ready for review July 3, 2020 05:04

thunderz99 changed the title ~~[WIP]feat: support cwd and env variable using sun jdi~~ feat: support cwd and env variable using sun jdi Jul 3, 2020

change envs config from Array to Map for more friendly user experience

37d80bd

thunderz99 mentioned this pull request Jul 3, 2020

feat: add cwd and env variables support fwcd/vscode-kotlin#42

Open

fwcd reviewed Jun 9, 2022

View reviewed changes

LewsTherinTelescope mentioned this pull request Dec 6, 2022

Incorrect current working directory fwcd/vscode-kotlin#112

Open

NecroKote mentioned this pull request Aug 7, 2024

support cwd, env and envFile parameters using sun jdi #90

Open

		return Bootstrap.virtualMachineManager().createVirtualMachine(connection,
		process)

		protected fun launchAndConnect(commandArray: Array<String>, listenKey: TransportService.ListenKey,
		ts: TransportService, cwd: String = "", envs: Array<String>? = null): Pair<Connection, Process>{

		class StringArgument constructor(private val name: String, private val description: String = "", private val label: String = name,
		private var value:String = "", private val mustSpecify: Boolean = false) : Connector.Argument {

		}


		} No newline at end of file

Conversation

thunderz99 commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Done

Linked pullreq

Uh oh!

bkleiner commented Apr 14, 2021

Uh oh!

fwcd left a comment

Choose a reason for hiding this comment

Uh oh!

fwcd Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thunderz99 commented Jul 2, 2022

Uh oh!

aharoldk commented Mar 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thunderz99 commented Jul 2, 2020 •

edited

Loading

fwcd Jun 9, 2022 •

edited

Loading