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

Add x:variable to schema and improve compiler support for x:variable #475

Merged
merged 74 commits into from
Aug 29, 2019

Conversation

galtm
Copy link
Member

@galtm galtm commented Dec 16, 2018

Per issue #60, x:variable is missing from the schema. This change adds it
and fixes some bugs in the compiler related to handling of variables.

A few questions and comments

  • I debated about x:variable interspersed with x:expect. It seems handy; for example, if you store the expected value in a variable, you might want the variable definition near the x:expect element that references the variable. However, if you have to put several x:expect elements inside x:pending, you won't want to have to move x:variable out of the way. Instead, you'll want x:pending to allow x:variable as a child. Since I've seen some issues with x:pending, I hesitate to allow x:variable inside x:pending at the moment. That could be a later enhancement, if there is enough interest. In practice, I rarely use x:pending with x:expect as a child.
  • The issue writeup is about both x:variable and x:apply. This pull request addresses x:variable only, and I have not done any work on x:apply.
  • Variables should be documented in one of the wikis for users. I will update the wiki soon after this pull request is merged in.

Per issue xspec#60, x:variable is missing from the schema. This change adds it
and fixes some bugs in the compiler related to handling of variables.
test/xspec-variable.xspec Outdated Show resolved Hide resolved
test/xspec-variable.xspec Outdated Show resolved Hide resolved
src/schemas/xspec.rnc Outdated Show resolved Hide resolved
test/xspec-variable.xspec Outdated Show resolved Hide resolved
test/xspec-variable.xspec Outdated Show resolved Hide resolved
@AirQuick

This comment has been minimized.

@galtm

This comment has been minimized.

@AirQuick

This comment has been minimized.

This was referenced Jan 25, 2019
@galtm

This comment has been minimized.

@AirQuick

This comment has been minimized.

@galtm

This comment has been minimized.

…ith last commit, fd5b4de

# Conflicts:
#	test/xspec-node-selection.xspec
#	test/xspec-variable.xspec
@AirQuick

This comment has been minimized.

…d align test/xspec-variable*.xspec with PR#320

# Conflicts:
#	test/xspec-variable.xspec
@cirulls

This comment has been minimized.

@galtm

This comment has been minimized.

…move tests requiring XSLT 3.0 from xspec-variable.xspec to xspec-eqname.xspec.
@galtm
Copy link
Member Author

galtm commented Aug 25, 2019

Regarding the 1.0 stylesheet, https://saxonica.plan.io/issues/4266 may help.

Thanks for the reference. I believe I don't need to do anything special to react to or mention XSLT 1.0 differences with regard to redefinition of variables. Redefined XSpec variables compile to redefined XSLT variables, which work fine in multiple recent Saxon versions because they operate as XSLT 2 or XSLT 3 processors.

Co-Authored-By: AirQuick <AirQuick@users.noreply.github.com>
test/xspec-node-selection.xspec Outdated Show resolved Hide resolved
test/xspec-node-selection.xspec Outdated Show resolved Hide resolved
galtm and others added 2 commits August 27, 2019 08:53
Co-Authored-By: AirQuick <AirQuick@users.noreply.github.com>
Co-Authored-By: AirQuick <AirQuick@users.noreply.github.com>
test/xspec-variable.xspec Outdated Show resolved Hide resolved
test/xspec-variable.xspec Outdated Show resolved Hide resolved
@AirQuick
Copy link
Member

I've reviewed all the files. We're quite close to the end of the game. 😃

@galtm
Copy link
Member Author

galtm commented Aug 28, 2019

I've reviewed all the files. We're quite close to the end of the game. 😃

Happy to hear that! Thanks, @AirQuick and @cirulls , for all your help and diligence along the way. :)

@AirQuick
Copy link
Member

#475 (comment)

  • I debated about x:variable interspersed with x:expect. It seems handy; ... I hesitate to allow x:variable inside x:pending at the moment. That could be a later enhancement, if there is enough interest. In practice, I rarely use x:pending with x:expect as a child.

I totally agree.

@AirQuick
Copy link
Member

What should we say about Schematron?
I vaguely remember @galtm mentioned somewhere that Schematron was out of scope. And I think it's perfectly ok. I'm just wondering about Release Note, Wiki or any other documentation.

@galtm
Copy link
Member Author

galtm commented Aug 28, 2019

When drafting the wiki content, I considered saying "XSLT and XQuery" explicitly somewhere. Then I saw that the entire Writing Scenarios wiki makes no mention of Schematron. How about having that wiki say near the top that the whole topic is about XSpec for XSLT and XQuery, and linking to the separate topic, Writing Scenarios for Schematron? Maybe something like the following, in the Introduction section:

Scenarios fall into four main types:

If there's a concise way to make the Release Notes indicate that the support for variables is in XSpec tests for XSLT and XQuery only, that sounds good.

@AirQuick AirQuick merged commit 4a3ae57 into xspec:master Aug 29, 2019
@AirQuick
Copy link
Member

How about having that wiki say near the top...

Sounds like a nice idea. Can you make those changes?

@galtm
Copy link
Member Author

galtm commented Aug 29, 2019

Yes, I updated the Writing Scenarios wiki with the extra Schematron bullet and the new XSpec Variables section. Thanks for merging this pull request!

@AirQuick
Copy link
Member

@cirulls
Merged this substantial improvement of x:variable (for XSLT and XQuery only), a long-standing but lesser-known feature.
I'd like the release note to highlight it and express big thanks to @galtm for this extraordinary contribution.

@cirulls
Copy link
Member

cirulls commented Aug 30, 2019

This is awesome! Thanks @galtm for the major contribution and @AirQuick for shepherding this improvement into the code base.

Absolutely, this will be the highlight of the next release. If you want to write a specific paragraph, post i here and I will include it in the release notes.

@galtm
Copy link
Member Author

galtm commented Aug 31, 2019

@AirQuick and @cirulls : You're very welcome, and thanks for the compliments. I enjoyed working with you both on this, and it was a super learning experience!

About the release notes, how about something like the following? Feel free to change anything, especially to make it fit in better with surrounding content. The 2nd sentence, in particular, can be removed if you think it is obvious or want to save space.

A useful enhancement in v1.4.0 is the ability to define your own variables in an XSpec test for XSLT or XQuery (#475). Reasons for using XSpec variables can include reusing data within the XSpec file and naming intermediate results for clarity. This feature had an incomplete implementation in earlier releases. For more information on using variables, see XSpec Variables.

(FYI, I will be away from the Internet until Tuesday.)

@cirulls
Copy link
Member

cirulls commented Sep 2, 2019

@galtm: it looks good to me, I added it to the draft release notes.

@galtm
Copy link
Member Author

galtm commented Sep 3, 2019

I added it to the draft release notes

Thanks, @cirulls !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants