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

Provide Script Names to Script Engines #1885

Merged
merged 7 commits into from Mar 2, 2021

Conversation

boc-tothefuture
Copy link

Per the spec

For instance, the value of the key
javax.script.filename
is defined to be the name of the file or resource containing the source
of the script. Setting this value makes the name of the source
available to the underlying scripting implementation for use in
constructing error messages.

I have tested this in JRuby and it works correctly.

With the ruby code:
puts __FILE__

Before the change:
<script>

After the change:
/src/openhab-scripting/tmp/openhab/conf/automation/jsr223/ruby/personal/test_1607028003.rb

@boc-tothefuture
Copy link
Author

FYI - The commit is signed off, the name just doesn't match the commit. OpenHAB requires the full name, where as the commit was not done with the full name.

@5iver
Copy link

5iver commented Dec 7, 2020

Duplicate of #1196. It looks like you also [refer a full path rather than a file name. Maybe with your input, this can finally be merged.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/jruby-openhab-rules-system/110598/2

@boc-tothefuture boc-tothefuture requested a review from a team as a code owner December 28, 2020 08:22
@cweitkamp cweitkamp added automation enhancement An enhancement or new feature of the Core duplicate labels Jan 16, 2021
Base automatically changed from master to main January 18, 2021 20:04
@kaikreuzer
Copy link
Member

@boc-tothefuture Is this PR still relevant? I see it has been marked as duplicate, although I am not sure if it isn't rather a further enhancement.
So please either close this PR or rebase it in order to resolve the merge conflict. Thanks!

@boc-tothefuture
Copy link
Author

I have rebased this.
@jpg0 - Are we Ok with this simple method of providing the filename to the script to assist in troubleshooting?

@jpg0
Copy link
Contributor

jpg0 commented Mar 1, 2021

Sure, fine by me. Personally I would prefer the setting to happen in the ScriptEngineManager as I don't think that it's the responsibility of the watcher to be concerned with javax.script stuff (and shouldn't tie itself to it), but it's not a big deal and could be fixed later. Same for the ability to use a filename rather than a filepath.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Thanks guys!

@kaikreuzer kaikreuzer merged commit 89796e7 into openhab:main Mar 2, 2021
@kaikreuzer kaikreuzer added this to the 3.1 milestone Mar 2, 2021
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
Signed-off-by: Brian O'Connell <broconne@gmail.com>
GitOrigin-RevId: 89796e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants