Al: defaultLaunchConfigurationName should not be stored in settings.json

Created on 16 Mar 2020  路  9Comments  路  Source: microsoft/AL

When you have more than one server configured in the launch.json file, and debug the app, VS Code asks which server you want to publish to... and stores its name as "al.defaultLaunchConfigurationName" in the settings.json file.

This is a bad practice: launch.json is a file that will be different depending on the developer but settings.json is defined as the settings for the project (not the user) and it is intended to be shared across all developers.

I'm not sure were this "al.defaultLaunchConfigurationName" should go, but it should be in a user owned file that is outside source control, not in file that is common to all developers.

bug in-progress workspaces

Most helpful comment

We are going to move this setting to a local workspace setting and add an opt-out in the debug configuration.
In a multi workspace scenario (I would assume that most implementations do use multi workspace by now) it is a per folder setting already.
Without this setting there is no go to definition in a scenario when one uses multiple debug configurations. And as we go ahead there will be more and more debug configuration types (launch, attach, time travel, etc). Moving to debug configuration file is not a viable solution, because how the AL debug adapter and the AL language server and communicating with an NST is designed.

All 9 comments

"al.defaultLaunchConfigurationName is set by default on the user settings for the per workspace folder.
"User settings => my settings". I would not put my settings in source control.

Interesting, I never had though that someone might use workspace settings for personal settings. I searched around after your comment, and there is a GitHub ticket for VS Code asking for a new settings.local.config file to do what you mention.

My previous experience with other languages/IDEs has been that workspace settings are always shared to keep everyone using the same settings that affect how we write code in a project/code generation/etc. (ex. analyzer rules for AL), so I didn't even consider this option, but I can see why someone might want to set some personal settings for a particular workspace.

Regardless, I think that my point stands: VS Code help explicitly states that this file can be shared across developers (and thus included in source control) and a per user workspace looks like its going to be implemented soon (it's mentioned in VS Code March 2020 plan) with a different name.

image

That's a cropped screenshot taken from this link: https://code.visualstudio.com/docs/getstarted/settings

I agree with salgiza. In an environment with multiple developers using the same workspace folder, not all developers must name theirs dockers equally.
Furthermore, it is mandatory to put something, because if not, it is asking for it constantly whenever Ctrl is pressed. When I indicate any environment, it installs the extension there by default, without asking. If I have several dockers to install it, I have to change it in settings. Before I could select where to install.
I think that, at least, it should be optional.

Explanation

While the launch.json is specific to developers and their respective environments, the workspace Settings are a classic area for project-specific settings and rules that apply to all developers. So the settings.json is not a good place to share developer specific settings.

While there are certainly situations where a developer will temporarily adjust settings.json, these settings are valid for everyone. Classically, the deactivation of rules that are not useful (for us) but must apply to everyone.

To summarize, saving user specific configuration in a workspace configuration file is not a good idea. I would suggest to apply the following changes:

Suggestions

  • Remove the setting from settings.json
  • Add a "default": true/false to the/each configuration or apply "defaultLaunchConfigurationName" to the root of launch.json, whatever fits best
  • Allow users to configure if the setting of the default launch configuration should be done (Always, Never, Ask, If Not Set)
  • And, as the icing on the cake, provide explicit "Publish to..." "Download from..." commands to override default settings on demand :)

Probably this can be aligned to [Feature] Local Workspace settings.

We are going to move this setting to a local workspace setting and add an opt-out in the debug configuration.
In a multi workspace scenario (I would assume that most implementations do use multi workspace by now) it is a per folder setting already.
Without this setting there is no go to definition in a scenario when one uses multiple debug configurations. And as we go ahead there will be more and more debug configuration types (launch, attach, time travel, etc). Moving to debug configuration file is not a viable solution, because how the AL debug adapter and the AL language server and communicating with an NST is designed.

The setting itself is a terrible idea.

Choosing the configuration for downloading symbols and launching was fast and convenient. Now that choice has been taken away from the user and they have to change the settings on the file to change the environment they are working on.

This makes zero sense and created a new problem where there wasn't one.

This has been fixed and will be available with the first update of BC 2020 spring release.
It will probably be available sooner in internal builds targeting the next version of BC.
There is no more defaultLaunchConfigurationName.
gotodefinition will rely on local dependency applications for resolving the source of a reference symbol for apps with runtime versions >= 5.0.
There is an extra feature that comes with it and that is DAL file symbol source navigation.
aka, gotodefinition within a DAL file defined symbol to its source (another DAL file).

For versions <= 4.0 gotodefinition will work the old way (as before defaultLaunchConfigurationName). No source file for reference symbols on a gotodefinition if more then one debug configuration is defined.

Unfortunately it did not make it to the 16.1 update of BC. Targeting 16.2 for now.

This issue should have been fixed as part of 16.2. If you can still reproduce it, please log a new issue with code reproducing the issue.

Was this page helpful?
0 / 5 - 0 ratings