-
Notifications
You must be signed in to change notification settings - Fork 422
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
Usage message customization (dependency injection and separation of concerns) #530
Comments
That's great feedback, thanks! I haven't looked at this area for a while. Let's see what we can do to improve things. There's a lot there, let me try to break it down:
interface ISectionRenderer {
String renderSection(CommandSpec spec, ColorScheme colorScheme);
}
static class Help {
// subclasses to override?
public List<ISectionRenderer> getRenderers() {
return Arrays.asList(
// default implementation returns current section ordering
(spec, scheme) -> headerHeading(),
(spec, scheme) -> header(),
(spec, scheme) -> synopsisHeading(),
(spec, scheme) -> synopsis(synopsisHeadingLength()),
// ...
);
}
public String getUsageHelpMessage() {
StringBuilder sb = new StringBuilder();
for (ISectionRenderer renderer : getRenderers()) {
sb.append(renderer.renderSection(commandSpec(), colorScheme());
}
return sb.toString();
}
...
} Then the
Thoughts? |
In the meantime, following your ideas I've just implemented the whole thing -- it's completely respectful of the existing API and idioms, yet flexible to customization, really sweet! Hard-coded rendering blocks have been refactored into their respective renderers, wiring back the existing entry points, so it's totally transparent to existing users. All the existing junit tests passed (in the first run, I only had to fix the headingLength used in the detailedSynopsis algorithm), so I'm confident to send you a pull request quite soon. |
…erers refactoring)
Hi Remko,
first of all, kudos for the huge work put in this project, it feels very accurate and full of dedication!
Despite all the goodies, I've bumped against an annoying issue: its API seems a bit tightly coupled, there's less support to dependency injection than one might expect. Conversely, it seems to lack separation between presentation and data.
Let me explain: in my case, I needed to slightly customize the usage message layout (embellishment tweaks (expansions, indentations...) to make it more pleasing to the eye).
I found out that the suggested route to expansions mixes line separation flags (
%n
) to actual contents (for example,descriptionHeading="%nDescription:%n%n"
), whilst indenting specific contents seems impossible without resorting to hackish tricks (publicCommandLine.usage(..)
methods don't acceptHelp
objects as argument, they all instantiate their own default implementation; unfortunately, the defaultHelp
class implementsdescription()
through a join that forcestable.indentWrappedLines=0
(sic!)).The possibility to inject a derived
Help
object throughCommandLine.usage(..)
would have spared me the annoying hack to extract the default layout implementation (CommandLine.usage(StringBuilder, Help)
) just to tweak its building.Furthermore, IMO, layout formatting within content attributes (such as surrounding headings with newlines to adjust their vertical spacing, as described above) should be discouraged, since presentation (as you certainly know) should be separate from actual data, delegating it as much as possible to renderers: each and every content member (headings, section contents (header, synopsis, description, footer...)...) should be associated to its own renderer, without hard-coding string manipulation as seen in current
Help
implementation. About section reordering, I'd throw away the hard-coded members currently exposed by theHelp
class, in favor of a flexible map of members fluently manipulatable without having to hard codeStringBuilder.append(..)
call trains (as seen inCommandLine.usage(StringBuilder, Help)
).Please, let me know if I missed/misinterpreted anything.
thank you!
The text was updated successfully, but these errors were encountered: