iljapostnovs / VSCodeUI5Plugin

Visual Studio Code Extension for UI5 Development
Apache License 2.0
61 stars 6 forks source link

Bug: UnusedTranslationLinter recognizes only "i18n.properties" file, but no other *.properties file #393

Closed richardatsap closed 7 months ago

richardatsap commented 8 months ago

Describe the bug The UnusedTranslationLinter marks only unused translations in an i18n.properties file. We split our translations into a couple of other files in folder "i18n" and for those neither the unused nor the duplicate key linter works anymore. This used to work until a couple of weeks ago.

To Reproduce Steps to reproduce the behavior:

  1. Create a "test.properties" file in some projects i18n folder (wherever i18n.properties is located).
  2. Put an unused key in the file. The key will not be marked as unused.
  3. Copy the key onto an additional line. The key will not be marked as duplicate.

Expected behavior As it has been in previous versions, all *.properties files should be scanned. If this is not desired, one should be able to either put an inclusion or an exclusion list into package.json.

Desktop (please complete the following information):

iljapostnovs commented 8 months ago

Hi,

Thanks for the report. I think that for i18n_*.properties files there could be additional linters created, which will compare the content with i18n.properties file. Maybe for i18n_*.properties there could be linters for: 1) Duplicates 2) If translation exists in i18n.properties but doesn't exist in i18n_*.properties 3) If translation exists in i18n_*.properties but doesn't exist in i18n.properties

Probably would be better way to go, I don't see any other way of linting other i18n files.

richardatsap commented 8 months ago

Hi,

my other properties files do not follow the pattern i18n_*.properties, so that fix wouldn't help.

Note, that this is a regression, in the past all my *.properties files where linted. Linting was within one file, not across. Don't know what you (or some dependency) changed in that regard.

If no other solution is possible, I could live with listing the *.properties files which should be linted in package.json. Linting doesn't need to be across files, within one file would be fine with me. In most cases, different files are used in different views.

iljapostnovs commented 8 months ago

Hi,

my other properties files do not follow the pattern i18n_*.properties, so that fix wouldn't help.

Note, that this is a regression, in the past all my *.properties files where linted. Linting was within one file, not across. Don't know what you (or some dependency) changed in that regard.

If no other solution is possible, I could live with listing the *.properties files which should be linted in package.json. Linting doesn't need to be across files, within one file would be fine with me. In most cases, different files are used in different views.

Hm, could you provide examples of what exactly kind of *.properties files you are using? I will think on the solution depending on that.

Yes, I changed the logic some time ago, because it was inconsistent. Long story short, there was an issue that if you go to e.g. 18n_lt.properties, it would override original i18n.properties and it would affect everything, e.g. i18n Code Lens in XML views, so yeah, I switched to the logic which works with i18n.properties only.

richardatsap commented 8 months ago

ui5-minimal.zip

Hi,

please have a look into the project attached. It contains a test.properties file in folder /webapp/i18n, which is bound to a model in the onInit method of MainApplication.controller.js (lines 46 ff.). The model is referred to in the respective view.

I'd prefer a solution which does not only tailor my specific needs but serves a wider audience. The previous behavior had the risk some false positives which I could live with. My main use case is to spot unused texts which arise often in a "living" application. A solution where I need to specify my i18n files in package.json would be fine with me.

iljapostnovs commented 7 months ago

Please check if latest version (1.16.6) works for you.

richardatsap commented 7 months ago

Works, thanks for the fix. May I ask what you changed? Did you revert?

Let me take this issue to say thanks for the great work!

iljapostnovs commented 7 months ago

Works, thanks for the fix. May I ask what you changed? Did you revert?

Let me take this issue to say thanks for the great work!

I reverted it but partly. So the primary issue was that I am caching i18n translations in one exemplar with app id as identifier. However, file watchers were configured to check all .properties files and if you go to other file it would override original i18n. As a result, if e.g. you go to i18n_de.properties file and to xml afterwards, it would show german translations instead of english from i18n.properties. As a solution I have changed the watchers to watch only for i18n.properties, which was a bad solution for projects with multiple i18ns. So now linting works without cache and is parsed on the fly for any .properties file, and only i18n.properties is cached, and the cache is used for i18n Code Lens.

I have plans to read all ResourceBundles from manifest.json and get model names, so I could add Code Lens for multiple i18n models, but I don't know yet when I will have time to implement it.