Suppress warning implementation from global settings #49

dariowskii · 2024-05-24T12:03:37Z

Fixes #48

You can suppress specific warnings (or all) by setting this values in settings.json:

"arbEditor.suppressedWarnings": <value> // "all" or [0, 1, ..., 9]

It's a good idea to open an issue first for discussion.

Implemented new tests
Tests pass
Appropriate changes to README are included in PR

mosuem · 2024-05-24T13:49:52Z

Thanks for the PR!

Although I suspect that dismissing warnings has usually some other bug at its root, which we should fix, it's still a good functionality to have. Unfortunately the CI seems to be broken, I will look into that!

dariowskii · 2024-05-24T16:38:59Z

Hi @mosuem,

Yes I agree, eliminating warnings can lead to bugs but in the end it is defined by the user, so it seems more like a calculated risk to me.

The reason that pushed me to create this PR is that I started defining my arb file like this:

{
     "@@locale": "en",

     "@_COMMON": {},
     "warning": "Warning",
     "error": "Error",
     "loading": "Loading",
     "close": "Close",
     "cancel": "Cancel",
     "done": "Done",

     "@_LOGIN_PAGE": {},
     "loginTitle": "Login Page"
}

Right now, @_COMMON and @_LOGIN_PAGE are diagnosed as errors and all the keys have warnings because they do not have the metadata defined (which in my opinion are not needed for simple and self-explanatory keys).
Having the possibility of eliminating some warnings without losing the other features offered by the plugin would be really convenient, and would also simplify reading the diagnosis tab!

(Anyway, really a strange behavior of CI 😅)

src/diagnose.ts

dariowskii · 2024-05-29T10:34:56Z

Hi @mosuem,

I pushed the update but noticed that the tests for quickfixes are not consistent. Testing locally I was able to notice that these (even without the integration of this PR) can arbitrarily fail or succeed.
I don't know if it depends on my computer or if there is something broken in the tests but I think they need to be revised.

mosuem · 2024-05-29T14:25:42Z

Testing locally I was able to notice that these (even without the integration of this PR) can arbitrarily fail or succeed.

That's not good :) Do you have any logs or similar which could help debugging this?

mosuem

LGTM (just a nit remaining, if you want to address that).

src/diagnose.ts

dariowskii · 2024-05-29T15:39:46Z

Testing locally I was able to notice that these (even without the integration of this PR) can arbitrarily fail or succeed.

That's not good :) Do you have any logs or similar which could help debugging this?

@mosuem Yes, I do. This is one log

    ✔ A rough parser test, as the real test will be done by the golden.
extensionHostProcess.js:147
    ✔ Test quickfix for missing Metadata (108ms)
extensionHostProcess.js:147
    1) Test quickfix for placeholder without metadata with tabs
extensionHostProcess.js:147
    ✔ Test quickfix for placeholder without metadata with spaces (110ms)
extensionHostProcess.js:147
    ✔ Test finding unescaped regions
extensionHostProcess.js:147
    ✔ Test suppressed all warnings (72ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with id missing_metadata_for_key (132ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with id 'invalid_key', 'missing_metadata_for_key' (47ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with code 'metadata_for_missing_key' (69ms)
extensionHostProcess.js:147
  10 passing (2s)
extensionHostProcess.js:147
  1 failing
extensionHostProcess.js:147
  1) Extension Test Suite
       Test quickfix for placeholder without metadata with tabs:
     Canceled
   Canceled
      at a.value (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:99:13540)
      at o.y (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:660)
      at o.z (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:730)
      at o.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:946)
      at R.cancel (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:5051)
      at N.cancel (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:5461)
      at Object.Z (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:4340)
      at c.N (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:4817)
      at c.L (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:3674)
      at a.value (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:2227)
      at o.y (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:660)
      at o.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:877)
      at u.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:107:14175)
      at a.value (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:176:8023)
      at o.y (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:660)
      at o.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:877)
      at u.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:107:14175)
      at MessagePortMain.<anonymous> (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:176:6303)
      at MessagePortMain.emit (node:events:517:28)
      at Object.MessagePortMain._internalPort.emit (node:electron/js2c/utility_init:2:2285)
      at Object.callbackTrampoline (node:internal/async_hooks:130:17)

extensionHostProcess.js:147
Error: 1 tests failed.
    at /Users/dariovarriale/Desktop/me/my_github/arb-editor/src/test/suite/index.ts:40:9
    at done (/Users/dariovarriale/Desktop/me/my_github/arb-editor/node_modules/mocha/lib/mocha.js:1012:7)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

Extension Test Suite
extensionHostProcess.js:147
    ✔ Decorate golden file. (340ms)
extensionHostProcess.js:147
    ✔ Decorate golden file with template. (259ms)
extensionHostProcess.js:147
    ✔ A rough parser test, as the real test will be done by the golden.
extensionHostProcess.js:147
    ✔ Test quickfix for missing Metadata (120ms)
extensionHostProcess.js:147
    1) Test quickfix for placeholder without metadata with tabs
extensionHostProcess.js:147
    ✔ Test quickfix for placeholder without metadata with spaces (72ms)
extensionHostProcess.js:147
    ✔ Test finding unescaped regions (107ms)
extensionHostProcess.js:147
    ✔ Test suppressed all warnings
extensionHostProcess.js:147
    ✔ Test suppressed warning with id missing_metadata_for_key (62ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with id 'invalid_key', 'missing_metadata_for_key' (45ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with code 'metadata_for_missing_key' (40ms)
extensionHostProcess.js:147
  10 passing (1s)
extensionHostProcess.js:147
  1 failing
  1) Extension Test Suite
       Test quickfix for placeholder without metadata with tabs:

      AssertionError [ERR_ASSERTION]: '{\n' +
  '\t"@@locale": "en",\n' +
  '\t"helloAndWelcome2": "Welcome {firstName}!",\n' +
  '\t"@helloAndWelcome2": {}\n' +
  '}' == '{\n' +
  '\t"@@locale": "en",\n' +
  '\t"helloAndWelcome2": "Welcome {firstName}!",\n' +
  '\t"@helloAndWelcome2": {\n' +
  '\t\t"placeholders": {\n' +
  '\t\t\t"firstName": {}\n' +
  '\t\t}\n' +
  '\t}\n' +
  '}'
      + expected - actual

       {
--
       	"helloAndWelcome2": "Welcome {firstName}!",
      -	"@helloAndWelcome2": {}
      +	"@helloAndWelcome2": {
      +		"placeholders": {
      +			"firstName": {}
      +		}
      +	}
       }
      
  	at compareGolden (/Users/dariovarriale/Desktop/me/my_github/arb-editor/out/test/suite/extension.test.js:159:16)
  	at async testFixAgainstGolden (/Users/dariovarriale/Desktop/me/my_github/arb-editor/out/test/suite/extension.test.js:147:5)
  	at async Context.<anonymous> (/Users/dariovarriale/Desktop/me/my_github/arb-editor/out/test/suite/extension.test.js:82:9)

extensionHostProcess.js:147
Error: 1 tests failed.
	at /Users/dariovarriale/Desktop/me/my_github/arb-editor/out/test/suite/index.js:35:27
	at done (/Users/dariovarriale/Desktop/me/my_github/arb-editor/node_modules/mocha/lib/mocha.js:1012:7)
	at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

That's another one

    ✔ A rough parser test, as the real test will be done by the golden. (136ms)
extensionHostProcess.js:147
    1) Test quickfix for missing Metadata
extensionHostProcess.js:147
    ✔ Test quickfix for placeholder without metadata with tabs (154ms)
extensionHostProcess.js:147
    ✔ Test quickfix for placeholder without metadata with spaces (84ms)
extensionHostProcess.js:147
    ✔ Test finding unescaped regions
extensionHostProcess.js:147
    ✔ Test suppressed all warnings (45ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with id missing_metadata_for_key (70ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with id 'invalid_key', 'missing_metadata_for_key' (64ms)
extensionHostProcess.js:147
    ✔ Test suppressed warning with code 'metadata_for_missing_key' (49ms)
extensionHostProcess.js:147
  10 passing (2s)
extensionHostProcess.js:147
  1 failing
extensionHostProcess.js:147
  1) Extension Test Suite
       Test quickfix for missing Metadata:
     Canceled
   Canceled
      at a.value (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:99:13540)
      at o.y (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:660)
      at o.z (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:730)
      at o.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:946)
      at R.cancel (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:5051)
      at N.cancel (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:5461)
      at Object.Z (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:4340)
      at c.N (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:4817)
      at c.L (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:3674)
      at a.value (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:150:2227)
      at o.y (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:660)
      at o.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:877)
      at u.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:107:14175)
      at a.value (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:176:8023)
      at o.y (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:660)
      at o.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:82:877)
      at u.fire (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:107:14175)
      at MessagePortMain.<anonymous> (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:176:6303)
      at MessagePortMain.emit (node:events:517:28)
      at Object.MessagePortMain._internalPort.emit (node:electron/js2c/utility_init:2:2285)
      at Object.callbackTrampoline (node:internal/async_hooks:130:17)

extensionHostProcess.js:147
Error: 1 tests failed.
    at /Users/dariovarriale/Desktop/me/my_github/arb-editor/src/test/suite/index.ts:40:9
    at done (/Users/dariovarriale/Desktop/me/my_github/arb-editor/node_modules/mocha/lib/mocha.js:1012:7)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

I also tried running the tests by disabling the extensions but the result is the same. Sometimes they all pass without problems, sometimes some tests for quickfixes fail or are canceled

dariowskii · 2024-06-08T07:41:55Z

Hi @mosuem, are there any updates?

mosuem · 2024-06-17T07:28:38Z

@dariowskii there is some test failure, if that is unrelated or fixed this is ready to merge.

dariowskii · 2024-06-17T07:49:49Z

@dariowskii there is some test failure, if that is unrelated or fixed this is ready to merge.

@mosuem I checked the test, I confirm that it is not related to the feature implemented in this PR 👍🏻

dariowskii · 2024-07-10T07:29:14Z

Hi @mosuem, will this update be published on the VS marketplace?

dariowskii added 3 commits May 24, 2024 13:51

Implemented arbEditor.supressWarnings key support in settings.json

32b2cc6

Updated README

a6fdafc

Implemented semicolons

a5efd5e

Merge branch 'main' into feature/supress-warnings

bcc80c2

mosuem reviewed May 28, 2024

View reviewed changes

src/diagnose.ts Outdated Show resolved Hide resolved

dariowskii added 4 commits May 29, 2024 12:27

Updated DiagnosticCode enum with string values

7f17309

Implemented configuration properties in package.json

df0611f

Updated tests

ac8e7c7

Updated README

52ca545

mosuem approved these changes May 29, 2024

View reviewed changes

src/diagnose.ts Show resolved Hide resolved

Clean old diagnostics when setting "all"

0e53d0a

mosuem mentioned this pull request Jun 10, 2024

Tests failing locally #51

Open

mosuem merged commit 9dd53e5 into google:main Jun 19, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suppress warning implementation from global settings #49

Suppress warning implementation from global settings #49

dariowskii commented May 24, 2024 •

edited

Loading

mosuem commented May 24, 2024

dariowskii commented May 24, 2024

dariowskii commented May 29, 2024

mosuem commented May 29, 2024

mosuem left a comment

dariowskii commented May 29, 2024 •

edited

Loading

dariowskii commented Jun 8, 2024

mosuem commented Jun 17, 2024

dariowskii commented Jun 17, 2024

dariowskii commented Jul 10, 2024

Suppress warning implementation from global settings #49

Suppress warning implementation from global settings #49

Conversation

dariowskii commented May 24, 2024 • edited Loading

mosuem commented May 24, 2024

dariowskii commented May 24, 2024

dariowskii commented May 29, 2024

mosuem commented May 29, 2024

mosuem left a comment

Choose a reason for hiding this comment

dariowskii commented May 29, 2024 • edited Loading

dariowskii commented Jun 8, 2024

mosuem commented Jun 17, 2024

dariowskii commented Jun 17, 2024

dariowskii commented Jul 10, 2024

dariowskii commented May 24, 2024 •

edited

Loading

dariowskii commented May 29, 2024 •

edited

Loading