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

refactor: test:generate-variable-declarations mode #782

Merged
merged 24 commits into from
Feb 19, 2020

Conversation

AirQuick
Copy link
Member

@AirQuick AirQuick commented Feb 14, 2020

In an attempt to make a fix of #777, I found these difficulties in mode="test:generate-variable-declarations":

  • The responsibility of analyzing context is dispersed.
  • The variable names being generated are sometimes hard-coded, sometimes dynamic.
  • Its code path from the initiator to the outcome is a winding road. Several modes and some temporary nodes are involved.
  • Its parameters are not named consistently.

I guess this complexity grew over time while integrating the independent test: module into XSpec and splitting XSLT and XQuery.

To mitigate it, this pull request refactors mode="test:generate-variable-declarations":

  • Now mode="test:generate-variable-declarations" owns the responsibility of analyzing context.
  • Now the variable names being generated are obtained from test:variable-name().
  • The intermediate modes/nodes have been eliminated.
  • All the parameters except for the tunnel $pending have been eliminated.

No actual change in the compiled stylesheets/queries.

Besides the automated tests, I manually compared the compiled stylesheets/queries with the current master using this command

for %I in ( test\xspec-node-selection.xspec test\xspec-variable.xspec ) do ( bin\xspec.bat "%~I" && bin\xspec.bat -q "%~I" && bin\xspec.bat "%~dpnI_stylesheet%~xI" )

and verified that only generate-id()-driven variable names were different.

@AirQuick AirQuick added this to the v1.6.0 milestone Feb 14, 2020
@commit-lint
Copy link

commit-lint bot commented Feb 14, 2020

Code Refactoring

  • relocate language-specific parameters (27e6f69)
  • wrap if-then-else (bb685b4)
  • take attribute(name) by default (e616a80)
  • remove xsl:for-each which is no longer necessary (4348948)
  • xslt: default $type based on context element (de59dcc)
  • rename $var to $name (53a73d7)
  • xquery: rename $global to $is-global (b866e96)
  • rename $var-doc to $temp-doc-name (ea4d486)
  • rename $var-doc-uri to $temp-uri-name (16a87d0)
  • xslt: rename $type to boolean $is-param (5397f40)
  • rename $variable-is-pending to $is-pending (7122874)
  • enable test:generate-variable-declarations to exclude child nodes (b7b6603)
  • fall back on generate-id() by default (79913b1)
  • use common function to get variable names (7a13fa4)
  • xquery: determine $is-global in callee (19730b4)
  • xslt: determine $is-param in callee (a5e8e66)
  • wrap some long lines (39bbf98)
  • remove x:generate-declarations which is no longer necessary (9aa6025)

Documentation

  • add comments to some xsl:param and xsl:variable (789c709)
  • xquery: adjust indent in comment (fa820a6)

Contributors

@AirQuick

@AirQuick
Copy link
Member Author

There may be a better refactoring idea; even so, I believe it is better to start from this one than from the current code. So, merging this. I'll soon submit a fix for #777 and ask someone's review.

@AirQuick AirQuick merged commit 55daa9b into xspec:master Feb 19, 2020
@AirQuick AirQuick deleted the refactor-vardecl-tmpl branch February 19, 2020 18:41
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

1 participant