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

Keyval Improvements and xkeyval support #860

Merged
merged 3 commits into from Aug 29, 2017

Conversation

Projects
None yet
3 participants
@tkw1536
Contributor

tkw1536 commented Aug 4, 2017

Tl;DR: This PR adds major improvements to the handling of KeyValue arguments, mostly based on the xkeyval package.

I have been working on this for the past few weeks and I am going to try to summarize the changes that I have made. I probably forgot something, so please have a look at the diffs as well.

The KeyVals logic, previously contained in KeyVals.pm and partially in TeX.pool.ltxml and Package.pm, has been completely refactored and split into two files KeyVals.pm and KeyVal.pm.

  • KeyVal.pm provides a new KeyVal class. This gives access to a single key, allowing it to be read and set. The file also contains a refactored DefKeyVal implementation (previously found in Package.pm. It is fully backwards compatible, so no rewriting of existing bindings is necessary. With the new class, DefKeyVal behaves much like its native TeX counterpart would. Furthermore, this refactoring adds extended support for the xkeyval package, such as command, choice and boolean keys.
  • KeyVals.pm provides the KeyVals class. Each instance of it represents a set of Key-Value pairs. This class has also been refactored, making the code cleaner and adding support for advanced xkeyval features such as key prefixes, setting keys in more than one family, and more.
    Unfortunately, the constructor of the KeyVals class is not completely backward compatible, so amongst some others, the listings package binding had to be slightly updated. The new constructor is fully documented with POD and making the change for other bindings (if any) should be straightforward.
  • In TeX.pool.ltxml, the RequiredKeyVals and OptionalKeyVals argument types have been given additional arguments and variants with '*' and '+' have been added. This is again based on the xkeyval package, and has been documented using in-line comments. The existing argument types are fully backward compatible.
  • keyval package bindings have updated to match the new KeyVals class.
  • xkeyval and xkvview package bindings have been added.
    • support for defining and setting KeyVals using this package
    • support for \DeclareOptionX and friends, which is an extended version of the LaTeXs \DeclareOption, both for sty and cls files.
  • lots and lots of test cases.

This does not support the pointer system of xkeyval. It is also not possible to preset keys. I plan to make a second pull request for these in the near future.

/cc @brucemiller

@dginev

This comment has been minimized.

Collaborator

dginev commented Aug 4, 2017

This sounds like truly great work!

lots and lots of test cases.

And I can't celebrate this enough! 🎉

@tkw1536 tkw1536 force-pushed the tkw1536:keyval branch 2 times, most recently from d117d45 to 0530606 Aug 27, 2017

@tkw1536

This comment has been minimized.

Contributor

tkw1536 commented Aug 27, 2017

Pull requested rebased and updated after offline suggestions from @brucemiller:

  • made sure that the first argument in DefPrimitive is called $stomach even though it is unused.
  • renamed test cases in t/keyval_dox to t/keyval_options for clarity
@brucemiller

This comment has been minimized.

Owner

brucemiller commented Aug 28, 2017

Looks good, except that you didn't change to keyval_options in the MANIFEST(?)

@tkw1536 tkw1536 force-pushed the tkw1536:keyval branch 2 times, most recently from 75f25cc to 6835e38 Aug 28, 2017

tkw1536 added some commits Aug 4, 2017

Proper 'keyval' package support
This commit extends support for the keyval package by extending the
existing bindings to be able to read key-value definitions and the
associated defaults from raw macro definitions. Furthermore, it prepares
the implementation for support of extended packages, like 'xkeyval' and
friends.

Furthermore, it adds a new group of tests 'keyval'. For now, this
contains two tests, `keyvalinline` and `keyvalstyle`. The first of these
is testing the new functionality, the second one is an old test that was
moved out of the graphics package.
Add 'xkeyval' bindings
This commit adds bindings for the xkeyval package which work without
relying on the .sty file. These bindings do not support pre-setting keys
or using the built-in pointer system.

Furthermore, this commit refactors and extends the KeyVals class, to be
compatible with the new types of key-value pairs defined in the xkeyval
package. It also adds tests for all appropriate cases.
Add 'xkvview' bindings
This commit adds bindings for the xkvview utility package along with a
very basic test case. The bindings are very light-weight and only read
in the sty file.

@tkw1536 tkw1536 force-pushed the tkw1536:keyval branch from 6835e38 to 131c4e5 Aug 28, 2017

@tkw1536

This comment has been minimized.

Contributor

tkw1536 commented Aug 28, 2017

Oops, I have fixed that now.

@brucemiller

This comment has been minimized.

Owner

brucemiller commented Aug 29, 2017

Looks good... and impressive! Thanks!

@brucemiller brucemiller merged commit f424387 into brucemiller:master Aug 29, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tkw1536 tkw1536 deleted the tkw1536:keyval branch Aug 29, 2017

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