stevedlawrence commented on code in PR #1452:
URL: https://github.com/apache/daffodil-vscode/pull/1452#discussion_r2417396514
##########
debugger/src/main/scala/org.apache.daffodil.debugger.dap/Utils.scala:
##########
@@ -70,3 +72,64 @@ object DataLeftOverUtils {
leftOverBitsText + dataHex + dataText
}
}
+
+/** Download and extract utils */
+object DownloadExtractUtils {
Review Comment:
> So would you prefer I delete the download and extracting from the Scala
code?
I just don't see how that could work since whatever it downloads won't end
up on the classpath without re-execing it self which I didn't see. So any stuff
using an Daffodil API would just end up with a ClassNotFound exception.
> This would mostly be in an effort to support the user running the debugger
directly and not only relying on the extension to run it for them
Is users running the debugger directly something that we expect to happen?
To me, that feels like something something I would only expect developers to
do, in which case I would suggest that requiring a path to a daffodil version
is not only reasonable but actually a good idea--we probably shouldn't guess
what daffodil version devs want to test with now that it supports multiple
versions.
But I'm also not too familiar with how the debugger is used. I'm not against
those kinds of changes if you think users will use it, or if it would be too
much of a pain for devs to download and/or specify additional options. I just
want to make sure we're adding features that people will use--every bit of code
we add needs to be tested and maintained so the less complex the better. Only
supporting one place where daffodil can be downloaded makes things a bit more
simple and avoids that additional burden, even if it's only a handful of lines
of code.
Another option is do implement the simple core stuff now, and if it really
does become a pain or users really do need to execute things outside of the
extension, those features can always be added later. It might be something hard
to consider if it's useful or not until this is merged and people actually
starting use this feature and playing with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]