-
Notifications
You must be signed in to change notification settings - Fork 789
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
Feature/streaming keys #69
Conversation
* Key key = LabeledSymbol('x', 'A', 5); // cast to key type | ||
* cout << key_formatter(MultiRobotKeyFormatter) << StreamedKey(key); | ||
*/ | ||
class key_formatter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Class naming is inconsistent and potentially confusing with KeyFormatter
. Unless this is more of a interface
class which is what it seems to be, then the name could be something like KeyFormatterFactory
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because it is a stream manipulator
static void *&property(std::ios_base &s); | ||
static void set_property(std::ios_base &s, const KeyFormatter &f); | ||
static KeyFormatter *get_property(std::ios_base &s); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for not updating the Boost typedef to a full-blown class with all these properties. The boost::function
function pointer can be a member of the class. Though this may be a significant engineering effort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The properties are not part of KeyFormatter
, but static methods belonging to the stream manipulator class key_formatter
, whose only purpose in life is to store that property in the stream.
Thanks for the comments. BTW, unless you feel very strongly about things (I don't think you did in this case - comments were more questions) or you're dealing with newbies (I'm not) just approve with comments in future PRs :-) |
5ddaff8ba Merge pull request #77 from borglab/fix/template-as-template-arg 0b6f2d92b allow templates as paramters to templates 7f3e242b0 Merge pull request #76 from borglab/fix/cmake-config 0caa79b82 macro to find and configure matlab 522557232 fix GTWRAP_INCLUDE_NAME 78a5d3afa Use CMakePackageConfigHelpers to vastly simplify the package config 76f8b9e5d Merge pull request #75 from borglab/fix/template-args 3b8e8389e remove reference from shared pointers 045393c7b docs and flag renaming d23d8beae tests ef96b4bdc don't make template parameters as references d1e1dc697 Merge pull request #74 from borglab/fix/type-recursion 8202ecf10 minor fixes 5855ea85b support for passing templated types as arguments 150cc0578 Support for templated return types 5c58f8d03 Merge pull request #73 from borglab/fix/types-refactor c697aa9c8 refactored the basic and custom types to make it cleaner, added more tests 98e2c3fa1 Merge pull request #68 from borglab/fix/cmake c6d5e786a make config agnostic to install prefix 4d6999f15 Merge pull request #69 from borglab/feature/call-and-index ccf408804 add support for callable and indexing overloads 8f8e3ec93 add status messages 88566eca4 make WRAP_PYTHON_VERSION an optional argument 01b8368ad Merge pull request #67 from borglab/feature/operator-overloading 522a12801 remove unsupported operators 209346133 update check location for unary operator 39e290f60 fix small typo faa589bec update DOCS 7ff83cec8 minor fixes 8ce37766f fixed tests 21c477c4d include pybind11/operators a3534ac5e wrap operator overloads 67c8f2089 instantiate templates for operators e9dce65d8 use ReturnType for ease in other places and use members in Class 3078aa6db added parser rule for operator overloading git-subtree-dir: wrap git-subtree-split: 5ddaff8bab6c05e8a943c94993bf496e13296dd6
Added streaming for Keys/Symbols and special
key_formatter
stream manipulator