You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2022/03/10 05:25:41 UTC
[GitHub] [daffodil-vscode] arosien opened a new pull request #87: Always use the current schema location for the breakpoint.
arosien opened a new pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87
Mistakenly hard-coded the schema as always the top-level one. Related to Multi-file Breakpoint #66.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] arosien commented on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
arosien commented on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1064296737
> @arosien I get 8 errors when running sbt universal:packageBin on this PR, are you seeing this as well?
I think I missed a portion of the commit... but the checks have passed. Hmm.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] tuxji merged pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
tuxji merged pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] arosien commented on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
arosien commented on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1065544189
Looks like this can be merged, but I don't have permission :(
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] mbeckerle commented on a change in pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#discussion_r823823941
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -328,15 +328,14 @@ object Parse {
/** Translate parse events to updated Daffodil state. */
def fromParse(
- schemaPath: Path,
frameIds: Next[DAPodil.Frame.Id],
variableRefs: Next[DAPodil.VariablesReference]
): Stream[IO, Parse.Event] => Stream[IO, DAPodil.Data] =
events =>
events.evalScan(DAPodil.Data.empty) {
case (_, Parse.Event.Init(_)) => IO.pure(DAPodil.Data.empty)
case (prev, startElement: Parse.Event.StartElement) =>
- createFrame(schemaPath, startElement, frameIds, variableRefs).map(prev.push)
+ createFrame(startElement, frameIds, variableRefs).map(prev.push)
Review comment:
I see where the schemaPath was wrong and startElement is better, but is there no logic needed to insure the endElement event hasn't already popped the stack?
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -364,7 +362,12 @@ object Parse {
startElement.name.map(_.value).getOrElse("???"),
/* If sourceReference > 0 the contents of the source must be retrieved through
* the SourceRequest (even if a path is specified). */
- new Types.Source(schemaPath.toString, 0),
+ Try(Paths.get(URI.create(startElement.schemaLocation.uriString)).toString())
+ .fold(
+ _ =>
+ new Types.Source(startElement.schemaLocation.uriString, null, 0), // there is no valid path if the location is a schema contained in a jar file; see #76.
Review comment:
line too long.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] tuxji commented on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
tuxji commented on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1065545968
OK, I merged for you, @arosien
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] arosien commented on a change in pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
arosien commented on a change in pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#discussion_r823932035
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -328,15 +328,14 @@ object Parse {
/** Translate parse events to updated Daffodil state. */
def fromParse(
- schemaPath: Path,
frameIds: Next[DAPodil.Frame.Id],
variableRefs: Next[DAPodil.VariablesReference]
): Stream[IO, Parse.Event] => Stream[IO, DAPodil.Data] =
events =>
events.evalScan(DAPodil.Data.empty) {
case (_, Parse.Event.Init(_)) => IO.pure(DAPodil.Data.empty)
case (prev, startElement: Parse.Event.StartElement) =>
- createFrame(schemaPath, startElement, frameIds, variableRefs).map(prev.push)
+ createFrame(startElement, frameIds, variableRefs).map(prev.push)
Review comment:
You are correctly guessing the actual problem with #66! (But the serialization of pushes and pops isn't managed at this particular point; this is just the handler of the events that need to be correctly serialized.)
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] Shanedell commented on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1064361027
+1
I tested this locally and was able to set a breakpoint at line 500 of `editfact/src/main/resources/EDIFACT-Common/EDIFACT-Service-Segments-4.1.xsd` and it was hit by the debugger with it stopping in the proper file
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] Shanedell commented on a change in pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
Shanedell commented on a change in pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#discussion_r824134190
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -364,7 +362,12 @@ object Parse {
startElement.name.map(_.value).getOrElse("???"),
/* If sourceReference > 0 the contents of the source must be retrieved through
* the SourceRequest (even if a path is specified). */
- new Types.Source(schemaPath.toString, 0),
+ Try(Paths.get(URI.create(startElement.schemaLocation.uriString)).toString())
+ .fold(
+ _ =>
+ new Types.Source(startElement.schemaLocation.uriString, null, 0), // there is no valid path if the location is a schema contained in a jar file; see #76.
Review comment:
I agree to both side that this line does seem long but does not show up in the scala formatting. Does scalafmt not count comment string in the line width? Maybe can the comment can be moved above`new Types.Source(startElement.schemaLocation.uriString, null, 0),` instead of on the same line?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] Shanedell removed a comment on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
Shanedell removed a comment on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1064361027
+1
I tested this locally and was able to set a breakpoint at line 500 of `editfact/src/main/resources/EDIFACT-Common/EDIFACT-Service-Segments-4.1.xsd` and it was hit by the debugger with it stopping in the proper file
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] arosien commented on a change in pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
arosien commented on a change in pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#discussion_r823958351
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -364,7 +362,12 @@ object Parse {
startElement.name.map(_.value).getOrElse("???"),
/* If sourceReference > 0 the contents of the source must be retrieved through
* the SourceRequest (even if a path is specified). */
- new Types.Source(schemaPath.toString, 0),
+ Try(Paths.get(URI.create(startElement.schemaLocation.uriString)).toString())
+ .fold(
+ _ =>
+ new Types.Source(startElement.schemaLocation.uriString, null, 0), // there is no valid path if the location is a schema contained in a jar file; see #76.
Review comment:
The check passed: https://github.com/apache/daffodil-vscode/runs/5491197876?check_suite_focus=true. Maybe a local formatter difference?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] mbeckerle commented on a change in pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#discussion_r824007712
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -364,7 +363,12 @@ object Parse {
startElement.name.map(_.value).getOrElse("???"),
/* If sourceReference > 0 the contents of the source must be retrieved through
* the SourceRequest (even if a path is specified). */
- new Types.Source(schemaPath.toString, 0),
+ Try(Paths.get(URI.create(startElement.schemaLocation.uriString)).toString())
Review comment:
So, not so much about this bug, but the jar-file limiations.....
You are having to jump through hoops like this here, because startElement doesn't give you anyway to get the jar file URL that you could open to get the source when that element's decl is coming from a jar. The schemaLocation is likely some absolute path that has to do with when directory structure when the jar was created, not how to retrieve the source from that jar file.
Let's think of what exactly you would want startElement to have on it to make this debug front end as simple as possible, then we can figure out how to make it easy to get that.
I think you need a URI/URL that you can do openConnection on to grab the source of the schema file, and you need the relative path to it - for labeling it meaningfully on the display of files without having a bunch of URI-jar artifacts in the name, and finally, you do need a status of exactly what jar that came from, and the fact that it came from a classpath jar, not just a directory.
Basically, I am suggesting we enhance the SchemaFileLocation object to have exactly the needed information on 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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] Shanedell commented on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1064666953
> > > @arosien I get 8 errors when running sbt universal:packageBin on this PR, are you seeing this as well?
> >
> >
> > I think I missed a portion of the commit... but the checks have passed. Hmm.
>
> I don't think the GitHub checks do anything with the Scala files of the repo. I'll file an issue about it.
The GitHub checks do check Scala code for formatting issues (https://github.com/apache/daffodil-vscode/runs/5499758056?check_suite_focus=true). Or are you referring to building the code doing something like `sbt universal:packageBin` to test 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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] Shanedell edited a comment on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
Shanedell edited a comment on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1064666953
> > > @arosien I get 8 errors when running sbt universal:packageBin on this PR, are you seeing this as well?
> >
> >
> > I think I missed a portion of the commit... but the checks have passed. Hmm.
>
> I don't think the GitHub checks do anything with the Scala files of the repo. I'll file an issue about it.
The GitHub checks do check Scala code for formatting issues (https://github.com/apache/daffodil-vscode/runs/5499758056?check_suite_focus=true). Or are you referring to building the code doing something like `sbt universal:packageBin` to test it? If you are referring to the latter I agree that should be added.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] arosien commented on a change in pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
arosien commented on a change in pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#discussion_r824220133
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -364,7 +363,12 @@ object Parse {
startElement.name.map(_.value).getOrElse("???"),
/* If sourceReference > 0 the contents of the source must be retrieved through
* the SourceRequest (even if a path is specified). */
- new Types.Source(schemaPath.toString, 0),
+ Try(Paths.get(URI.create(startElement.schemaLocation.uriString)).toString())
Review comment:
So far in practice the `schemaLocation.uriString` has proved valid for jar-based schemas. They look like `jar:file:/x/y/test.jar!/foo/bar.xsd` and parse correctly.
If anything might change in the schema location, it would be to use a `URI` instead of a `String`.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] arosien commented on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
arosien commented on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1064562876
> > @arosien I get 8 errors when running sbt universal:packageBin on this PR, are you seeing this as well?
>
> I think I missed a portion of the commit... but the checks have passed. Hmm.
I don't think the GitHub checks do anything with the Scala files of the repo. I'll file an issue about 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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] Shanedell commented on pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#issuecomment-1064182958
@arosien I get 8 errors when running `sbt universal:packageBin` on this PR, are you seeing this as well?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-vscode] arosien commented on a change in pull request #87: Always use the current schema location for the breakpoint.
Posted by GitBox <gi...@apache.org>.
arosien commented on a change in pull request #87:
URL: https://github.com/apache/daffodil-vscode/pull/87#discussion_r823944253
##########
File path: server/core/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala
##########
@@ -364,7 +362,12 @@ object Parse {
startElement.name.map(_.value).getOrElse("???"),
/* If sourceReference > 0 the contents of the source must be retrieved through
* the SourceRequest (even if a path is specified). */
- new Types.Source(schemaPath.toString, 0),
+ Try(Paths.get(URI.create(startElement.schemaLocation.uriString)).toString())
+ .fold(
+ _ =>
+ new Types.Source(startElement.schemaLocation.uriString, null, 0), // there is no valid path if the location is a schema contained in a jar file; see #76.
Review comment:
Hmm, I assumed the build would run the formatter, but maybe that isn't a valid assumption. Will check and fix.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org