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

Finalize draft not aware diagnostics for getRelatedListRecords and getRelatedListCount #147

Conversation

ben-zhang-at-salesforce
Copy link
Contributor

@ben-zhang-at-salesforce ben-zhang-at-salesforce commented Oct 30, 2024

  • Finalize the diagnostic UI for getRelatedListRecords and getRelatedListCount as below:
    image
  • Skip LSP diagnostics if opened project is not sfdx or opened document is not under LWC folder.

@ben-zhang-at-salesforce ben-zhang-at-salesforce requested a review from a team as a code owner October 30, 2024 00:42
@@ -45,22 +47,26 @@ export class AdaptersLocalChangeNotAware implements DiagnosticProducer<Node> {
start: textDocument.positionAt(item.start as number),
end: textDocument.positionAt(item.end as number)
},
message: LOCAL_CHANGE_NOT_AWARE_MESSAGE
message: LOCAL_CHANGE_NOT_AWARE_MESSAGE,
code: LOCAL_CHANGE_NOT_AWARE_EXTERNAL_DOC_URL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

diagnostic renders code and codeDescription as link

const document = documents.get(uri);

// Do diagnostics if document is under LWC folder and already in cache.
if (uri.indexOf(WorkspaceUtils.LWC_PATH) > 0 && document !== undefined) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only do diagnostics for file under lwc folder

@@ -45,10 +45,5 @@ export async function validateDocument(
results = results.concat(diagnostics);
}

// Set the source for diagnostic source.
results.forEach((diagnostic) => {
diagnostic.source = extensionName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, why did you get rid of this? I wasn't a fan of this post processing anyway so removing it is a welcome change but where is the source set now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The vscode has its own brain where and how to render the diagnostic attributes. I like to render the diagnostic.source "Salesforce Mobile Extension in ui so to tell developer where this diagnostic is from. When we try to render a link in diagnostic pop up like what Kevin wants. the source is rendered between the msg and cde/codeDescription like below. Totally messed up the effect we want. Removing the source makes the popup close to what we want.
image

Source will not be set if we will render the link.
Kevin is thinking of what will be finally word for the rules, if there's no link needed. we can put the source back.

Copy link
Contributor

Choose a reason for hiding this comment

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

To make myself clear, I wasn't the fan of post processing, so removing is fine I think. But do what you think is the best thing to do.

Bumps [micromatch](https://github.com/micromatch/micromatch) from 4.0.5 to 4.0.8.
- [Release notes](https://github.com/micromatch/micromatch/releases)
- [Changelog](https://github.com/micromatch/micromatch/blob/master/CHANGELOG.md)
- [Commits](micromatch/micromatch@4.0.5...4.0.8)

---
updated-dependencies:
- dependency-name: micromatch
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ben Zhang <ben.zhang@salesforce.com>
…updates (salesforce#149)

* fix(deps): bump the minor-and-patch group across 1 directory with 14 updates

Bumps the minor-and-patch group with 13 updates in the / directory:

| Package | From | To |
| --- | --- | --- |
| [yaml](https://github.com/eemeli/yaml) | `2.5.1` | `2.6.0` |
| [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) | `22.2.0` | `22.8.6` |
| [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) | `8.0.1` | `8.12.2` |
| [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) | `8.1.0` | `8.12.2` |
| [mocha](https://github.com/mochajs/mocha) | `10.7.3` | `10.8.2` |
| [@types/mocha](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/mocha) | `10.0.8` | `10.0.9` |
| [nyc](https://github.com/istanbuljs/nyc) | `17.0.0` | `17.1.0` |
| [ovsx](https://github.com/eclipse/openvsx/tree/HEAD/cli) | `0.9.2` | `0.10.0` |
| [@babel/traverse](https://github.com/babel/babel/tree/HEAD/packages/babel-traverse) | `7.25.7` | `7.25.9` |
| [@graphql-tools/graphql-tag-pluck](https://github.com/ardatan/graphql-tools/tree/HEAD/packages/graphql-tag-pluck) | `8.3.2` | `8.3.3` |
| [@babel/cli](https://github.com/babel/babel/tree/HEAD/packages/babel-cli) | `7.25.7` | `7.25.9` |
| [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) | `7.25.7` | `7.26.0` |
| [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) | `7.25.7` | `7.26.0` |



Updates `yaml` from 2.5.1 to 2.6.0
- [Release notes](https://github.com/eemeli/yaml/releases)
- [Commits](eemeli/yaml@v2.5.1...v2.6.0)

Updates `@types/node` from 22.2.0 to 22.8.6
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

Updates `@typescript-eslint/eslint-plugin` from 8.0.1 to 8.12.2
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.12.2/packages/eslint-plugin)

Updates `@typescript-eslint/parser` from 8.1.0 to 8.12.2
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.12.2/packages/parser)

Updates `mocha` from 10.7.3 to 10.8.2
- [Release notes](https://github.com/mochajs/mocha/releases)
- [Changelog](https://github.com/mochajs/mocha/blob/main/CHANGELOG.md)
- [Commits](mochajs/mocha@v10.7.3...v10.8.2)

Updates `@types/mocha` from 10.0.8 to 10.0.9
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/mocha)

Updates `nyc` from 17.0.0 to 17.1.0
- [Release notes](https://github.com/istanbuljs/nyc/releases)
- [Changelog](https://github.com/istanbuljs/nyc/blob/main/CHANGELOG.md)
- [Commits](istanbuljs/nyc@nyc-v17.0.0...nyc-v17.1.0)

Updates `ovsx` from 0.9.2 to 0.10.0
- [Release notes](https://github.com/eclipse/openvsx/releases)
- [Changelog](https://github.com/eclipse/openvsx/blob/master/cli/CHANGELOG.md)
- [Commits](https://github.com/eclipse/openvsx/commits/v0.10.0/cli)

Updates `@babel/traverse` from 7.25.7 to 7.25.9
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.25.9/packages/babel-traverse)

Updates `@graphql-tools/graphql-tag-pluck` from 8.3.2 to 8.3.3
- [Release notes](https://github.com/ardatan/graphql-tools/releases)
- [Changelog](https://github.com/ardatan/graphql-tools/blob/master/packages/graphql-tag-pluck/CHANGELOG.md)
- [Commits](https://github.com/ardatan/graphql-tools/commits/@graphql-tools/graphql-tag-pluck@8.3.3/packages/graphql-tag-pluck)

Updates `@graphql-tools/utils` from 10.5.4 to 10.5.5
- [Release notes](https://github.com/ardatan/graphql-tools/releases)
- [Changelog](https://github.com/ardatan/graphql-tools/blob/master/packages/utils/CHANGELOG.md)
- [Commits](https://github.com/ardatan/graphql-tools/commits/@graphql-tools/utils@10.5.5/packages/utils)

Updates `@babel/cli` from 7.25.7 to 7.25.9
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.25.9/packages/babel-cli)

Updates `@babel/core` from 7.25.7 to 7.26.0
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.26.0/packages/babel-core)

Updates `@babel/preset-env` from 7.25.7 to 7.26.0
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.26.0/packages/babel-preset-env)

---
updated-dependencies:
- dependency-name: yaml
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: "@types/node"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: "@typescript-eslint/eslint-plugin"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: mocha
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: "@types/mocha"
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: minor-and-patch
- dependency-name: nyc
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: ovsx
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: "@babel/traverse"
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: minor-and-patch
- dependency-name: "@graphql-tools/graphql-tag-pluck"
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: minor-and-patch
- dependency-name: "@graphql-tools/utils"
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: minor-and-patch
- dependency-name: "@babel/cli"
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: minor-and-patch
- dependency-name: "@babel/core"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
- dependency-name: "@babel/preset-env"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: minor-and-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix babel traverse error

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ben Zhang <ben.zhang@salesforce.com>
@ben-zhang-at-salesforce
Copy link
Contributor Author

I messed up the change while rebase, will close this one and create a new pr.

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.

3 participants