Update to NB-25

[Achal1607]

This PR aims to update LS to NB-25.

[sid-srini]

Thanks a lot @Achal1607 for upgrading the LS to NB25.

[sid-srini]

Shouldn't the vmArgs be treated conditionally, just like config.vmArgs?
i.e. check whether it is an array or a string. If there is a mismatch between the types of both, then the vmArgs needs to be converted to the type of the config.vmArgs. Right?
In case vmArgs is a string and is to be pushed into an array, then we should perhaps split using quote-aware whitespace-delimiters into an array before pushing. That would ensure that downstream usage of config.vmArgs does not cause the double-quoting of multiple args into a single arg, which would not be understood by the JVM.

[Achal1607]

vmArgs is always a string since it comes from the settings.json. In package.json you can find:

"jdk.runConfig.vmOptions": {
	"type": "string",
	"default": "",
	"description": "%jdk.configuration.vmOptions.description%"
}

So, we don't need to do any string manipulation there IMO.
launch.json can have array of vmArgs. For example:

{
    "version": "0.2.0",
    "configurations": [
        {
            "type": "jdk",
            "request": "launch",
            "name": "Launch Java App",
            "vmArgs": ["-Xms256m"]
        }
    ]
}

But the side panel can only have string type of vmOptions as it comes from the settings.json.

In future can also convert settings.json configuration to take input as array as well.

[sid-srini]

Ok. Then we would need to do just the following:

In case vmArgs is a string and is to be pushed into an array, then we should perhaps split using quote-aware whitespace-delimiters into an array before pushing. That would ensure that downstream usage of config.vmArgs does not cause the double-quoting of multiple args into a single arg, which would not be understood by the JVM.

[Achal1607]

@sid-srini I have addressed all the review comments. Please review it again. Thanks

[sid-srini]

Typically, this will be functionally correct, although the performance maybe improved by storing a regex reference and performing a search followed by concat of substr. However, in certain corner cases this may not give the correct result. For e.g. when there are a balanced number of escape backslashes which indicate a literal backslash instead:

• "abc \\" def should become 2 strings; whereas the current code would retain it as a single string.

[Achal1607]

Thanks for pointing out this edge case, will update the method.

[Achal1607]

@sid-srini Addressed review comments. Thanks

[sid-srini]

Thanks @Achal1607 for completing the upgrade to NB25 and also improving the implementation of some of the new functionality. LGTM 👍

[sid-srini]

Nit: an index out-of-bounds error may occur with the unchecked i-1 or i-2 index access. Safety or performance-wise, it may be better to add a separate else-if block for keeping track of number of continuous occurrences of char == '\\' i.e. isEscapingActive and just check that during quote handling.

[Achal1607]

i > 1 will check if index is greater than 1 if it is true then only javascript will check other conditions.

Please correct me if I perceived your comment incorrectly?