rokucommunity / roku-debug

A compatibility wrapper around the BrightScript debug protocol https://developer.roku.com/en-ca/docs/developer-program/debugging/socket-based-debugger.md
MIT License
13 stars 9 forks source link

Bugfix/do not alter out file path for libraries #112

Closed philanderson888 closed 1 year ago

philanderson888 commented 1 year ago

Regarding issue 106 https://github.com/rokucommunity/roku-debug/issues/106 I am creating a pull request.

This

1) does not alter the file path for library paths specified in XML script tags eg

<script type="text/brightscript" uri="common:/LibCore/v30/bslCore.brs"/>

these paths are not altered when generating the 'out' directory.

Normal files are altered by appending a '__lib0' (or similar) to the end.

2) create a test to verify that the above code is working.

philanderson888 commented 1 year ago

Over to you @TwitchBronBron thanks, from Phil.

philanderson888 commented 1 year ago

Thanks bron; made small amendments as suggested. For some reason the 'match' string is stored as a full string of this format

url="common:/....

so I had to amend your suggested code slightly test you wrote is passing.

TwitchBronBron commented 1 year ago

Thanks bron; made small amendments as suggested. For some reason the 'match' string is stored as a full string of this format

url="common:/....

so I had to amend your suggested code slightly test you wrote is passing.

The latest commit won't work for xml like uri = "common:/..." (notice the spaces around the equals).

In that case, we should run a regex test instead.

if(/^uri\s*=\s*"common:\//i.exec(match)) {

Also, add a few more script import lines in the unit test to handle spaces around equal signs

philanderson888 commented 1 year ago

OK suggested fix is

// only alter file ending if it is a) pkg:/ url or b) relative url 
let isPkgUrl = false
let isRelativeUrl = false                
if (/^uri\s*=\s*"pkg:\//i.exec(match)) {
    isPkgUrl = true
}
if (!(/:\//i.exec(match))) {
    isRelativeUrl = true 
}
if (isPkgUrl || isRelativeUrl){
    return match.replace('.brs', this.postfix + '.brs');
} else {
    return match;
}
philanderson888 commented 1 year ago

... and amended tests

describe('addPostFixToPath', () => {
    it('adds postfix if path is 1) pkg:/ or 2) relative - no spaces in url', async () => {

            <component name="CustomComponent" extends="Rectangle">
                <script type="text/brightscript" uri="common:/LibCore/v30/bslCore.brs"/>
                <script type="text/brightscript" uri="CustomComponent.brs"/>
                <script type="text/brightscript" uri="pkg:/source/utils.brs"/>
            </component>

    it('adds postfix if path is 1) pkg:/ or 2) relative - plus spaces in url', async () => {

            <component name="CustomComponent" extends="Rectangle">
                <script type="text/brightscript" uri = "common:/LibCore/v30/bslCore.brs"/>
                <script type="text/brightscript" uri = "CustomComponent.brs"/>
                <script type="text/brightscript" uri = "pkg:/source/utils.brs"/>
            </component>

});
philanderson888 commented 1 year ago

over to you guys @TwitchBronBron @chrisdp

philanderson888 commented 1 year ago

Not a problem guys; happy to help