search

rokucommunity / vscode-brightscript-language

A Visual Studio Code extension for Roku's BrightScript language
MIT License
110 stars 40 forks source link

How can we use this with custom builds? Is there a mapping option? #8

Closed georgejecook closed 5 years ago

georgejecook commented 5 years ago

Hi,

This is similar to https://github.com/TwitchBronBron/vscode-brightscript-language/issues/5; but slightly different. We use gulp to build different versions of our project (i.e. include test files/different config files, etc). Is there a way we can launch the app; but map it so I can run my gulp build task first, and have vscode-brightscript-language and roku-deploy inject the stop commands in the code in our build folder, which is an artifact of our build process?

This would allow us the flexibility to maintain our own process, and still use debugging in your excellent plugin.

tl;dr, instead of copying sources, adding stops and zipping it all up, can we add the stops to our own build output folder (which already has the copied sources)?

TwitchBronBron commented 5 years ago

I love the idea, and would be more than open to having this included in the project if needed!

I think vscode already supports this concept though? In the launch.json, there's a setting called preLaunchTask. You could use that to fire off a vscode task in tasks.json which runs your gulp build. Paired with roku-deploy's rootDir setting (to point to the build folder instead of your source folder), the debugger could then kick off all the debugger things.

The downside to this approach is that the breakpoints would need to be in the build folder's copy of the files. However, if gulp is tweaking/changing files, I don't know any other way to put the breakpoints in the right spots.

Thoughts?

georgejecook commented 5 years ago

The downside to this approach is that the breakpoints would need to be in the build folder's copy

Yes - this is it in a nutshell.. assuming my gulp task is not changing line offsets, what can we do? This will be the difference between me using your plugin or not next week. On a project crunch, so I can't invest oodles of time into working around this.. and I'm really really keen to dogfood this plugin and maximize it's utility. I've been using my definitions for a couple of days, and it's a game-changer.

I've been using your breakpoint debugger and ditto - seems a logical match to put them together and start bolstering functionality.. (indentation/function information/navigate to xml compoenents etc).. there's a lot we can quickly add, so I'm really 🤞 you know of some easy way I can get started with getting the plugin to add stops to my gulp task output in the build folder.. you got any suggestions/quick hack? I don't care if it's held together by sticky tape :) only asking in case you know something quick that will save me having to wade/read through that bit of code, while I'm still hot on the other aspects. Hope that comes across as intended; might sound like I'm horse-trading with you, which is not my intention ;)

TwitchBronBron commented 5 years ago

So, assuming for a moment that all of the files in src are in the exact same locations as dist, I think this would be an easy addition to the extension. Roku reports debug file names relative to the root of the package. So, when I get that debug file name, I convert that path back to the workspace folder path (which is the "rootDir" variable in the launch configuration).

I can add another launch config variable called "debugRootDir" which will override "rootDir" only during the debugging phase.

The caveat here is, if you have files in src that get moved to different locations in dist. For example:

"src/tests/file.brs" being built and placed at "dest/file.brs", that file would not be debuggable with this fix. It gets you 90% of the way there though, and we can discuss approaches for that edge case.

Do you think that will work for most of your files?

TwitchBronBron commented 5 years ago

I just pushed a commit to the repo that I think solves this for any files that have the exact same file contents and file path in source and destination. Pull it down and let me know what you think.

georgejecook commented 5 years ago

cool! and thanks a bunch. I'm checking it out now. not sure if I have it as intended; because I still see the same thing - none of my fonts are in the built app, which to me suggests that it wasn't using the output of the prebuild task (which I've verified is working).. here's my config


      "type": "brightscript",
      "request": "launch",

      "preLaunchTask": "gulp: build",
      "name": "BrightScript Debug: Launch",
      "host": "192.168.16.56",
      "password": "aaaa",
      "rootDir": "${workspaceFolder}/src", 
      "debugRootDir":"${workspaceFolder}/build/default"
    }```

I've got your changes, too.. I've not dived into them in depth, coz truth is I'm knee deep in getting my code merged into yours and adding some new features..

It looks to me like roku-deploy is not deploying what's in the build folder (i.e. output from my gulp build task).. perhaps I'm just misconfiguring?
TwitchBronBron commented 5 years ago

You have the folders backwards. rootDir will be your build folder, and debugRootDir will be your src folder.

On Sat, Nov 17, 2018, 21:31 George Cook <notifications@github.com wrote:

cool! and thanks a bunch. I'm checking it out now. not sure if I have it as intended; because I still see the same thing - none of my fonts are in the built app, which to me suggests that it wasn't using the output of the prebuild task (which I've verified is working).. here's my config

  "type": "brightscript",
  "request": "launch",

  "preLaunchTask": "gulp: build",
  "name": "BrightScript Debug: Launch",
  "host": "192.168.16.56",
  "password": "aaaa",
  "rootDir": "${workspaceFolder}/src",
  "debugRootDir":"${workspaceFolder}/build/default"
}```

I've got your changes, too.. I've not dived into them in depth, coz truth is I'm knee deep in getting my code merged into yours and adding some new features..

It looks to me like roku-deploy is not deploying what's in the build folder (i.e. output from my gulp build task).. perhaps I'm just misconfiguring?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-439662845, or mute the thread https://github.com/notifications/unsubscribe-auth/ACbTbWpLKJLNZpaa7SmfnvJAJuybfBXeks5uwMadgaJpZM4YnNeF .

georgejecook commented 5 years ago

Thanks. I'll try that and let you know.

On Sun, Nov 18, 2018, 7:28 AM Bronley Plumb <notifications@github.com wrote:

You have the folders backwards. rootDir will be your build folder, and debugRootDir will be your src folder.

On Sat, Nov 17, 2018, 21:31 George Cook <notifications@github.com wrote:

cool! and thanks a bunch. I'm checking it out now. not sure if I have it as intended; because I still see the same thing - none of my fonts are in the built app, which to me suggests that it wasn't using the output of the prebuild task (which I've verified is working).. here's my config

"type": "brightscript", "request": "launch",

"preLaunchTask": "gulp: build", "name": "BrightScript Debug: Launch", "host": "192.168.16.56", "password": "aaaa", "rootDir": "${workspaceFolder}/src", "debugRootDir":"${workspaceFolder}/build/default" }```

I've got your changes, too.. I've not dived into them in depth, coz truth is I'm knee deep in getting my code merged into yours and adding some new features..

It looks to me like roku-deploy is not deploying what's in the build folder (i.e. output from my gulp build task).. perhaps I'm just misconfiguring?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-439662845 , or mute the thread < https://github.com/notifications/unsubscribe-auth/ACbTbWpLKJLNZpaa7SmfnvJAJuybfBXeks5uwMadgaJpZM4YnNeF

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-439689220, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKIUXYjoznwgOCO9WhDyW_S7i5OSrvTks5uwVJrgaJpZM4YnNeF .

TwitchBronBron commented 5 years ago

@georgejecook did this work for you (I'm anxious to know!).

georgejecook commented 5 years ago

Will let you know presently.

On Sun, Nov 18, 2018, 10:16 PM Bronley Plumb <notifications@github.com wrote:

@georgejecook https://github.com/georgejecook did this work for you (I'm anxious to know!).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-439759600, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKIUfvWW3AVWIswFq-5yIWuHxXAZgX4ks5uwiJ6gaJpZM4YnNeF .

georgejecook commented 5 years ago

Took some debugging; but I worked it out, and it works perfectly: t h a n k s !!

I needed add this to my config (you might want to add this to your docs, if you document the feature)

"files": [ "", ".*", "*/.*", ],

Thanks!

George

On November 19, 2018 at 8:56:05 AM, George Cook (george@tantawowa.com) wrote:

Will let you know presently.

On Sun, Nov 18, 2018, 10:16 PM Bronley Plumb <notifications@github.com wrote:

@georgejecook https://github.com/georgejecook did this work for you (I'm anxious to know!).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-439759600, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKIUfvWW3AVWIswFq-5yIWuHxXAZgX4ks5uwiJ6gaJpZM4YnNeF .

TwitchBronBron commented 5 years ago

With my simple example locally, I was able to use this feature without needing to do that. Would you be able to provide a sample repo that showcases why you needed to override the files array?

georgejecook commented 5 years ago

I’ll try get you something over the next couple of days - it’s tough coz I’ve worked around it, and I’d need to make a custom repo.. you’ll likely get this next week tbh - seeing as it’s non-critical for me.

On November 19, 2018 at 4:11:17 PM, Bronley Plumb (notifications@github.com) wrote:

With my simple example locally, I was able to use this feature without needing to do that. Would you be able to provide a sample repo that showcases why you needed to override the files array?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-440042898, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKIUQBQFrsqGH1sjYgqg9sytvH3LFgSks5uwx50gaJpZM4YnNeF .

pixshatterer commented 5 years ago

@TwitchBronBron would love to see this already available!

georgejecook commented 5 years ago

I think this is already merged @pixshatterer - check the commit history - you can use the interim build until @TwitchBronBron pushes out tonight

georgejecook commented 5 years ago

should this be closed @TwitchBronBron ? It's working fine for me.

TwitchBronBron commented 5 years ago

@georgejecook I left it open because you mentioned needing to use a catchall wildcard list for the files array, whereas for my test project I didn't need to override those at all. I was hoping you would provide some type of sample project that would let me investigate why that was needed, and fix the bug if possible.

georgejecook commented 5 years ago

Ah sorry I forgot that.

I'll get you a sample project in the future; I'm afraid thats not high priority for me and replicating a large client project is not an insignificant task. Perhaps better close this and have a more specific ticket for the actual issue? That way you might get others respond.. IMHO you resolved this original ticket and this is a distinct issue. For example, what if every time you deliver on this another. Issue arises, will we just keep this incorrectly titled ticket open as agrab bag of feature and bug creep? Its your repo; but I wouldn't consider this good sdp in a pro setting for the reasons I've laid out here and more.

On Tue, Dec 4, 2018, 6:50 AM Bronley Plumb <notifications@github.com wrote:

@georgejecook https://github.com/georgejecook I left it open because you mentioned needing to use a catchall wildcard list for the files array, whereas for my test project I didn't need to override those at all. I was hoping you would provide some type of sample project that would let me investigate why that was needed, and fix the bug if possible.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-444073542, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKIUQ3e5vXULlsDla2M8PdYNVrtRerUks5u1mGFgaJpZM4YnNeF .

georgejecook commented 5 years ago

And on that note; I should've raised a distinct ticket.. I didn't because I thought my workaround is fine.. if you want I can raise that. Let me know.

On Tue, Dec 4, 2018, 8:04 AM George Cook <george@tantawowa.com wrote:

Ah sorry I forgot that.

I'll get you a sample project in the future; I'm afraid thats not high priority for me and replicating a large client project is not an insignificant task. Perhaps better close this and have a more specific ticket for the actual issue? That way you might get others respond.. IMHO you resolved this original ticket and this is a distinct issue. For example, what if every time you deliver on this another. Issue arises, will we just keep this incorrectly titled ticket open as agrab bag of feature and bug creep? Its your repo; but I wouldn't consider this good sdp in a pro setting for the reasons I've laid out here and more.

On Tue, Dec 4, 2018, 6:50 AM Bronley Plumb <notifications@github.com wrote:

@georgejecook https://github.com/georgejecook I left it open because you mentioned needing to use a catchall wildcard list for the files array, whereas for my test project I didn't need to override those at all. I was hoping you would provide some type of sample project that would let me investigate why that was needed, and fix the bug if possible.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TwitchBronBron/vscode-brightscript-language/issues/8#issuecomment-444073542, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKIUQ3e5vXULlsDla2M8PdYNVrtRerUks5u1mGFgaJpZM4YnNeF .

TwitchBronBron commented 5 years ago

Yeah, go ahead and raise another issue so we can track it.