openhab / openhab-js

openHAB JavaScript Library for JavaScript Scripting Automation
https://www.openhab.org/addons/automation/jsscripting/
Eclipse Public License 2.0
38 stars 31 forks source link

[items] Add missing semantic actions to `ItemSemantics` #252

Closed rkoshak closed 1 year ago

rkoshak commented 1 year ago

Fixes #251.

Adds the missing Semantic actions to the ItemSemantics class.

rkoshak commented 1 year ago

If you are finished with your changes, run a npm run lint:fix to apply linting. If you want to run all checks that CI is doing on pull requests, run npm run deploy.

I'll look into it more, but both commands fail for me. It might be because I'm running it in a WSL2 environment.

You need to import items, adding this to the top should work:

I think I need to come up with a better workflow. Git won't allow me to have a repo under a repo (or I've not figured out how to do that) so I edit the files in place under $OH_CONF and then copy the files or make the changes again on the actual git clone. Clearly I forgot that line when making the edits the second time.

The rest of the comments have been addressed in the latest commits.

florian-h05 commented 1 year ago

I'll look into it more, but both commands fail for me. It might be because I'm running it in a WSL2 environment.

For me it worked in WSL2 when I was using it some time ago (I‘m on Linux for a while now).

Can you try to run npm install first? If that doesn‘t help, please post your error message.

I think I need to come up with a better workflow. Git won't allow me to have a repo under a repo (or I've not figured out how to do that) so I edit the files in place under $OH_CONF and then copy the files or make the changes again on the actual git clone.

I usually do the same if I encounter any errors while testing my changes.

Clearly I forgot that line when making the edits the second time.

The linter would have noticed that. FYI: The code is still working for you, because by default items is injected into the script, but this can be disabled so the import is needed.

rkoshak commented 1 year ago

I figured out why running npm deploy and fix:linting didn't work. I needed to install snazzy separately.

florian-h05 commented 1 year ago

It is defined as dev dependency so npm should install it.

Can you please regenerate the type definitions? npm run types And then test them: npm run types:test

They are required for the great IntelliSense in VS Code (you need to have the npm package installed).

If TypeScript errors because it doesn‘t know the Item, adding this to the top of the file should help:

/**
 * @typedef {import('./items').Item} Item
 * @private
 */
rkoshak commented 1 year ago

It is defined as dev dependency so npm should install it.

The first time I tried to run npm run lint:fix it did indeed install a bunch of stuff. But apparently not snazzy. Once I installed that by hand the command started working. Not sure why.

Can you please regenerate the type definitions? npm run types And then test them: npm run types:test

Ran clean with no errors. Should I check in the two files that changed?

florian-h05 commented 1 year ago

Ran clean with no errors. Should I check in the two files that changed?

If both npm run types and npm run types:test ran clean without errors, you can commit the changes files from the types folder.

Can you please update the CHANGELOG.md file, use items as namespace and the PR title without the [] part as description. I guess you’ll get how to fill the rest of that table line for this PR.