HackStudio: New project dialog + Project templates #5213
Conversation
nvella
force-pushed
the
hackstudio-new-project
branch
2 times, most recently
from
5b1e90d
to
4a712fe
Compare
|
Feedback has been applied @linusg :) I've also since added a rudimentary shared library template, and we're also now automatically opening the project after successful creation. |
There was a problem hiding this comment.
I really like this - very good feature for both newcomers and quick project creation, using the shell for postcreate scripts seem like it's going to be very versatile, and the UI is sleek !
Just a few feedbacks, I don't think any of them are blocking, though :)
| if (project_template->create_project(maybe_project_name.value(), maybe_project_full_path.value())) { | ||
| // Succesfully created, attempt to open the new project | ||
| m_created_project_path = maybe_project_full_path.value(); | ||
| done(ExecResult::ExecOK); | ||
| } else { | ||
| GUI::MessageBox::show_error(this, "Could not create project: internal error. Please check your parameters and try again."); | ||
| } |
There was a problem hiding this comment.
How about making project_template return an AK::Result<String, String> instead, with the first being the created project full path (for easier unwrapping below), and the second being a more detailed error string ? "Internal Error" is not as descriptive as we could be, and it seems like create_project could return a better message pretty easily.
There was a problem hiding this comment.
I may very well be missing something obvious, but it seems it's impossible to specify an error value for a Result with the same value and error type, which stops us from passing through the unwrapped project full path. It is possible to construct a Result<void, String> however, still allowing us to pass through an error string. It doesn't seem to be much functionally differently from a Optional<string>, but it seems like a more explicit way to pass an error than just using an Optional.
| if (maybe_project_path.has_value()) { | ||
| m_full_path_label->set_text(maybe_project_path.value()); | ||
| } else { | ||
| m_full_path_label->set_text("Invalid name or creation directory."); | ||
| m_input_valid = false; | ||
| } |
There was a problem hiding this comment.
Same as for the project template, having the project path be an Option<String> instead of a Result<String, String> limits the message we can show to the user ("Invalid" isn't as useful as we could be, given that the get_project_full_path() method can fail in 6 different ways both by itself or in get_available_project_name()).
There was a problem hiding this comment.
Referring to the previous conversation, it seems I'll have to pass on this for now until I find out how to specify an error value for Results with the same value and error type 😄 Shouldn't be too much of a problem for now.
There was a problem hiding this comment.
Wow, that's right ! I hadn't even considered the possibility thiat it couldn't be done, and that seems like an unfortunate limitation of the Result type.
I don't have any issues with skipping it for this PR. It's just a small usability thing that can be improved later once the Result<T, T> thing is possible!
There was a problem hiding this comment.
An alternative would be to have a :
struct TemplateCreationError {
TemplateCreationError(String message)
: message(message)
{
}
String message;
}and use it as the error type. This way, you could write return TemplateCreationError("My message");
There was a problem hiding this comment.
Hmm... I've managed to pass along the template creation error with a Result<void, String> so far, do you think it would be more correct defining a specific error struct for this circumstance, even though only a String is being passed at the moment?
There was a problem hiding this comment.
If you hold on just a few minutes I'll push my new changes up (unsquashed) :)
There was a problem hiding this comment.
I don't actually know if creating a specific struct for errors is good practice in C++.
@awesomekling thoughts about using Result<void, String> vs Result<void, TemplateCreationError>, where TemplateCreationError is a wrapper struct around a string ?
|
Very cool work! Love this feature. ⭐ |
No worries, and thanks for the review 😄 I'm usually quite busy during the week, I should be able to get around to all of it Friday evening (upside-down time 😆) |
nvella
force-pushed
the
hackstudio-new-project
branch
from
f4cd256
to
faaeff4
Compare
|
Just pushed my commits addressing the reviews :) |
In some circumstances (like template selection dialogs,) displaying as much item label as possible, on all items, may be desired. The default setting is 'false', which matches the default behaviour from before; only wrapping on hover or selection.
This commit adds a simple project template system to HackStudio, as well as a pretty New Project dialog, inspired by early VS.NET and MS Office.
nvella
force-pushed
the
hackstudio-new-project
branch
from
faaeff4
to
88be8c8
Compare
Decided to get this up as a draft to get some running feedback as I work on it, commits aren't squashed yetAll features done, rebased onto master and tested working - ready for a proper review 😃Summary
New Project dialog for HackStudio which reads simple templates from
/res/devel/templates. Templates consist of an INI file (template manifest,) at the moment containing just name and description texts, a directory containing the template contents, and optionally a post-creation script which is ran at create-time with the project name and path as arguments. Four templates are included; empty, C++ basic (command-line), C++ GUI (a simple Hello World example), and C++ shared library. Icons are stored in/res/icons/hackstudio/templates-32x32and share the same name as the template manifest INI and content directory.What's done
ProjectTemplateclass andProjectTemplateManagersingleton classProjectTemplate::create_projectmethod, where the magic happens.What I'm not entirely sure about
Still getting the hang of the general concept of smart pointers, I come from the land of managed memory (.NET) so go easy on me. 😅 There's probably a few mistakes around my pointer choices, object allocation methods and so on - constructive criticism is very much appreciated :)