Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: workspace codelens #1075

Merged
merged 12 commits into from
May 28, 2021
Merged

Conversation

sandikbarr
Copy link
Contributor

adds nx run targets CodeLens to the workspace config

@nx-cloud
Copy link

nx-cloud bot commented May 21, 2021

Nx Cloud Report

CI ran the following commands for commit 4d6e53f. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --all --parallel --target=build
#000000 nx run-many --all --parallel --target=lint
#000000 nx run-many --all --parallel --target=test

Sent with 💌 from NxCloud.

@sandikbarr sandikbarr changed the title Feat: workspace codelens feat: workspace codelens May 21, 2021
@@ -42,6 +45,10 @@ import {
NxProjectTreeItem,
NxProjectTreeProvider,
} from '@nx-console/vscode/nx-project-view';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved verifyWorkspace from from the verify lib to the nx-workspace lib to resolve a circular dependency between the two.

* Provides a CodeLens set for a matched document
* @param document a document matched by the pattern passed to registerCodeLensProvider
* @returns ProjectCodeLens Range locations and properties for the document
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

took out async on this function because it was no longer returning a Promise

@sandikbarr sandikbarr marked this pull request as ready for review May 25, 2021 14:49
'nx.run',
(project?: string, target?: string, configuration?: string) => {
let flags;
if (configuration) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with -c configuration here because ng run on an angular cli project doesn't take --prod as an argument, but it does accept -c. Technically, the CodeLens is formatted as nx|ng run project:target:configuration which is likely the most preferred form, but I wanted to pass configuration as flags and run it in the same fashion as these other cli task commands.

Comment on lines 274 to 304
function registerWorkspaceCodeLensProvider(context: ExtensionContext) {
if (GlobalConfigurationStore.instance.get('enableWorkspaceConfigCodeLens')) {
codeLensProvider = languages.registerCodeLensProvider(
{ pattern: '**/{workspace,angular}.json' },
new WorkspaceCodeLensProvider()
);
context.subscriptions.push(codeLensProvider);
}
}

function watchWorkspaceCodeLensConfigChange(context: ExtensionContext) {
context.subscriptions.push(
workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => {
// if the `nxConsole` config changes, check enableWorkspaceConfigCodeLens and register or dispose
if (
event.affectsConfiguration(
GlobalConfigurationStore.configurationSection
)
) {
const enableWorkspaceConfigCodeLens = GlobalConfigurationStore.instance.get(
'enableWorkspaceConfigCodeLens'
);
if (enableWorkspaceConfigCodeLens && !codeLensProvider) {
registerWorkspaceCodeLensProvider(context);
} else if (!enableWorkspaceConfigCodeLens && codeLensProvider) {
codeLensProvider.dispose();
codeLensProvider = null;
}
}
})
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This looks like it works properly 😄

Do you think this logic should be handled in the workspace lib? Like have a main entry to that lib, and then do all this logic in there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Can't keep throwing everything in main/activate.

resolveCodeLens(lens: CodeLens): CodeLens | Promise<CodeLens> | null {
if (lens instanceof ProjectCodeLens) {
const command: Command = {
command: 'nx.run',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to change to 'ng.run' for angular workspaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't actually implement ng.run 😬 because I didn't realize at the time we'd also be able to use that with angular cli workspaces as well. I should add it and use it here as well though; won't be too much. Good point.

selectCliCommandAndShowUi('generate', context.extensionPath)
);

commands.registerCommand(`${cli}.generate.ui.fileexplorer`, (uri: Uri) =>
Copy link
Contributor Author

@sandikbarr sandikbarr May 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The context menu generate and run commands were also just implemented in the Nx category. All of our other commands are implemented both as nx and ng. So this latest commit should now make these more consistent.

  • Right click Nx generate (ui) was working on Angular CLI projects, but it was just an "nx" VS Code command.
  • Adding a separate command for ng.generate.ui.fileexplorer gives us finer control over that menu item, so I made it ng generate (ui) for Angular CLI projects. This change from Nx generate (ui) in the context menu for Angular CLI only projects makes sense to me, but lmk if you think that's not minor enough to slip it in here.
  • I also added ng run to the context menu for Angular CLI projects that uses the Command Palette and prompts

configuration?: string
) {
let flags: string[] | undefined;
if (configuration) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I actually finally settled on --configuration=production because that will resolve the same as the other run command tasks to give us that "Task is already active" if something is already running with the same command and args.

@@ -117,6 +124,9 @@ export function activate(c: ExtensionContext) {
manuallySelectWorkspaceDefinitionCommand
);

// registers itself as a CodeLensProvider and watches config to dispose/re-register
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's this? I have this registering itself as a CodeLensProvider and watching the config settings for changes to dispose/re-register?

@sandikbarr
Copy link
Contributor Author

Thank you for your feedback on this @Cammisuli. I've been using it on a few different workspaces and am feeling pretty confident in it, but lmk if you see anything else that should be addressed. 🎉

@Cammisuli Cammisuli merged commit 3e18e99 into nrwl:master May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants