-
Notifications
You must be signed in to change notification settings - Fork 192
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
Fix #3831 - New (labs) CLI command for measure new
and measure copy
#4875
Conversation
343867f
to
7b14e7e
Compare
struct MeasureNewOptions | ||
{ | ||
std::string name; | ||
std::string className = "MyExampleMeasure"; | ||
openstudio::path directoryPath; | ||
std::string taxonomyTag = "Envelope.Fenestration"; | ||
openstudio::MeasureType measureType = openstudio::MeasureType::ModelMeasure; | ||
openstudio::MeasureLanguage measureLanguage = openstudio::MeasureLanguage::Ruby; | ||
std::string description = "DESCRIPTION_TEXT"; | ||
std::string modelerDescription = "MODELER_DESCRIPTION_TEXT"; | ||
|
||
void debug_print() const; | ||
}; |
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.
Struct (and its defaults) for new/copy is here
@@ -57,6 +73,8 @@ namespace cli { | |||
bool run_tests = false; | |||
|
|||
unsigned server_port = 0; | |||
|
|||
MeasureNewOptions newMeasureOpts; |
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 added to here. because we use make_shared for scoping issues... don't worry about it
// TODO: this includes 'UtilityMeasure' which I don't think we want to include | ||
std::vector<std::string> measureTypeNames; | ||
{ | ||
const auto& m = openstudio::MeasureType::getNames(); | ||
std::transform(m.cbegin(), m.cend(), back_inserter(measureTypeNames), [](const auto& x) { return x.second; }); | ||
// fmt::print("measureTypeNames={}\n", measureTypeNames); | ||
[[maybe_unused]] auto* measureTypeOpt = | ||
newMeasureSubCommand | ||
->add_option("-t,--type", opt->newMeasureOpts.measureType, | ||
fmt::format("Type of Measure [Default: '{}']: {}", opt->newMeasureOpts.measureType.valueName(), measureTypeNames)) | ||
->option_text("measureType") | ||
->check(CLI::IsMember(measureTypeNames)); | ||
} |
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.
TODO
->add_option("-t,--type", opt->newMeasureOpts.measureType, | ||
fmt::format("Type of Measure [Default: '{}']: {}", opt->newMeasureOpts.measureType.valueName(), measureTypeNames)) | ||
->option_text("measureType") | ||
->check(CLI::IsMember(measureTypeNames)); |
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.
Throughout the options, I'm going to validate input.
eg try to pass measure new --type Badval
and you'll get:
--type: Badval not in {ModelMeasure,EnergyPlusMeasure,UtilityMeasure,ReportingMeasure}
[[maybe_unused]] auto* directoryPathOpt = | ||
newMeasureSubCommand->add_option("DIRECTORY", opt->newMeasureOpts.directoryPath, "Directory for the measure") | ||
->option_text(" ") | ||
->required(true) | ||
->check(CLI::NonexistentPath); |
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.
Throw if the target directory exists. BCLMeasure's ctor to create a new one would throw anyways, but this throws earlier and a lot more clearly.
$ mkdir ./test_measure && openstudio labs measure new --class-name MyExamplePythonMeasure ./test_measure
DIRECTORY: Path already exists: ./test_measure
{ | ||
static constexpr auto helpOptionsGroupName = "Help"; | ||
static constexpr auto newMeasureExamples = R"(Examples: | ||
|
||
1. Create a Ruby ModelMeasure: | ||
|
||
``` | ||
$ openstudio labs measure new --class-name MyExampleRubyModelMeasure | ||
$ openstudio labs measure new --class-name MyExampleRubyModelMeasure --type ModelMeasure --language Ruby | ||
``` | ||
|
||
2. Pass all optional args to create a Python EnergyPlusMeasure: | ||
|
||
``` |
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.
Helpers, because I feel like this is going to be more than helpful given the number of options we can optionally pass
$ openstudio labs measure new --list-for-first-taxonomy-tag HVAC | ||
``` | ||
)"; | ||
newMeasureSubCommand->set_help_flag("-h,--help", "Print this help message and exit")->group(helpOptionsGroupName); |
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.
Since I'm making a specific "Help" group, might as well make the CLI11 default '--help' fall into that group.
[[maybe_unused]] auto* listTaxonomyFlag = newMeasureSubCommand | ||
->add_flag_callback( | ||
"--list-taxonomy-tags", | ||
[taxonomyTags]() { | ||
fmt::print("{}\n", taxonomyTags); | ||
std::exit(0); // NOLINT(concurrency-mt-unsafe) | ||
}, | ||
"List all available Taxonomy tags") | ||
->group(helpOptionsGroupName); |
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.
Couple of flags for taxonomy lookup, because even though I could just try to pass openstudio measure new --taxonomy-tag Badval ./test_measure
and look at the error message to get accepted values, this is clearer to use openstudio measure new --list-taxonomy-tags
Also added one that looks up from a first level tag: openstudio measure new --list-for-first-taxonomy-tag Envelope
(and throws if that first tag isn't valid)
NOTE: maybe this should live onto the measure
subcommand and not the measure new
subsubcommand, but that's maybe nitpicking, and it would reduce the readability of the help message in measure
subcomand
if (opt->newMeasureOpts.name.empty()) { | ||
opt->newMeasureOpts.name = opt->newMeasureOpts.className; | ||
} |
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.
If the "--name" isn't passed, default to the required --class-name
one
copyMeasureSubCommand->callback([opt] { | ||
boost::optional<BCLMeasure> b_; | ||
try { | ||
b_ = BCLMeasure(opt->directoryPath); | ||
} catch (...) { | ||
fmt::print(stderr, "Could not find a valid measure at '{}'\n", openstudio::toString(opt->directoryPath)); | ||
std::exit(1); | ||
} | ||
auto bClone_ = b_->clone(opt->newMeasureOpts.directoryPath); | ||
if (bClone_) { | ||
// Force updating the UID | ||
bClone_->changeUID(); | ||
bClone_->checkForUpdatesFiles(); | ||
bClone_->checkForUpdatesXML(); | ||
bClone_->save(); | ||
fmt::print("Cloned the {} {} with class name '{}' from '{}' to '{}'\n", b_->measureLanguage().valueName(), b_->measureType().valueName(), | ||
b_->className(), openstudio::toString(openstudio::filesystem::canonical(b_->directory())), | ||
openstudio::toString(openstudio::filesystem::canonical(bClone_->directory()))); | ||
std::exit(0); // NOLINT(concurrency-mt-unsafe) | ||
} else { | ||
fmt::print(stderr, "Something went wrong when cloning the measure"); | ||
std::exit(1); // NOLINT(concurrency-mt-unsafe) | ||
} |
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.
Copying a measure. Exit gracefully if the directory isn't a valid measure.
Make sure we do end up with a new UID and everything is up to date in the measure.xml.
7b14e7e
to
db89fba
Compare
I’m reluctant to add new CMake OpenStudio/src/cli/CMakeLists.txt Line 88 in 8054887
Or we’re ok not adding explicit tests for these and do it manually, as they shouldn't change often anyways. |
…_utilities_minimal
… directory that isn't a valid measure
37ac6a0
to
3b5ea39
Compare
CI Results for 3b5ea39:
|
Pull request overview
measure --new
andmeasure --copy
#3831measure --help
openstudio labs measure --help
measure new --help
Subhelp
$ openstudio labs measure new --examples
Examples:copy --help
Pull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.