-
Notifications
You must be signed in to change notification settings - Fork 24
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 #262 #263 - Handle Path Arguments (QFileDialog) #761
base: develop
Are you sure you want to change the base?
Conversation
here is my test measure
|
// TODO: aside from the fact that these arguments are weird / incorrectly named, neither the BCL-gem schema nor OS SDK actually handle them so | ||
// they are not even inside the measure.xml | ||
bool isRead = argument.get("is_read", Json::Value(false)).asBool(); | ||
std::string extension = argument.get("extension", Json::Value("*")).asString(); | ||
std::string extension = argument.get("extension", Json::Value("All files (*)")).asString(); |
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.
Default to something sensible for now
filed at NREL/OpenStudio#5273
} else if (argument.type() == measure::OSArgumentType::Path) { | ||
m_step.setArgument(argument.name(), openstudio::toString(argument.valueAsPath())); |
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.
Without this you, you get valueAsString, and it gets quoted in the workflowJSON as
"my_path_argument": "\"/the/path/\""
and when you run, Ruby for eg can't understand it.
// TODO: sanitize the input? | ||
m_extension = toQString(extension); |
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 here. Basically I think OS SDK should store it as a readable format, and we can reconstruct a proper Qt files Filter from it.
class PathInputView : public InputView | ||
{ | ||
Q_OBJECT | ||
|
||
public: | ||
PathInputView(const std::string& extension, bool isRead); | ||
virtual ~PathInputView() = default; | ||
|
||
QLineEdit* lineEdit; | ||
QPushButton* selectPathButton; | ||
|
||
void setName(const std::string& name, const boost::optional<std::string>& units, const boost::optional<std::string>& description); | ||
|
||
void setIncomplete(bool incomplete) override; | ||
|
||
void setDisplayValue(const QVariant& value) override; | ||
|
||
signals: | ||
void selectedPathChanged(const openstudio::path& p); | ||
|
||
private slots: | ||
void onSelectPathButtonClicked(); | ||
|
||
private: | ||
QLabel* nameLabel; | ||
QString m_extension; | ||
bool m_isRead; |
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.
New PathInputView class
The OS SDK / BCL-gem implementation of the path arguments is incomplete and confusing, and they don't handle the makePathArgument arguments:
isRead
. what does that even mean? Is this assuming that this is for reading meaning it must be 1) a file (not a directory) and 2) it must exist?)extension