Skip to content
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

Add --plugins argument to dita command line #1960 #3037

Merged
merged 4 commits into from Aug 9, 2018

Conversation

robander
Copy link
Member

@robander robander commented Aug 3, 2018

Signed-off-by: Robert D Anderson robander@us.ibm.com

Description

Adds a new command line arguments --plugins and --transtypes to the dita command line. When used, they provide a list of installed plug-in IDs and transtype names, one per line.

Pretty sure my implementation could be improved, but I won't know until I open the pull request and wait for the comments...

Motivation and Context

Requested in #1960 along with follow-up discussion.

There is also a request for an option that would display a list of transform types, but that is not (yet) part of this request - waiting to see how the --plugins implementation goes over.

How Has This Been Tested?

Tested by running the command line bin/dita --plugins and bin/dita --transtypes, verifying that each installed plugin is listed.

Type of Changes

  • New feature (non-breaking change which adds functionality)

Checklist

Have not updated unit tests - I don't see unit tests for the other command line parameters so not sure where this one should go.

@robander robander added feature New feature or request CLI Command-line interface issues labels Aug 3, 2018
@robander robander requested a review from jelovirt August 3, 2018 15:17
@jelovirt jelovirt added this to In progress in 3.2 via automation Aug 6, 2018
@jelovirt jelovirt moved this from In progress to Review in 3.2 Aug 6, 2018
@jelovirt jelovirt moved this from Review to In progress in 3.2 Aug 6, 2018
private void printPlugins() {
final List<String> installedPlugins = getInstalledPlugins();
if (installedPlugins != null) {
System.out.println("The following DITA-OT plugins are installed:");
Copy link
Member

@jelovirt jelovirt Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I want to have this line output and each ID prefixed with spaces. My preference would be to just have one ID per line, without any prefixes. That way the output could be piped to other processes without parsing out the pretty printing.

This is basically plumbing vs porcelain distinction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong feeling on it, either is fine.

When I got the first one working it didn't have spaces, and that seemed to look a little odd in my DOS shell. I put in the 3 spaces because they matched the --help output. But ... yeah it's easy to convince me to remove them here.

Copy link
Member

@jelovirt jelovirt Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is, for what is the output used for. In the original request

This would also allow for scripts that can check dependencies and current installation status in support of coordinated installation of multiple plugins.

This sounds like request for plumbing output.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robander should we then change the output be plumbing output, i.e. only output the plug-in IDs or trantype names, without prefix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's probably a good idea -- just generate the list, one per line, no extra spacing.

@jelovirt
Copy link
Member

jelovirt commented Aug 6, 2018

Added support for listing trantypes from CLI, IMO that's in the same class or requirement as listing plugins. @robander are you ok with this addition?

Robert D Anderson and others added 3 commits August 6, 2018 18:56
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Since the list is used only once, it doesn't make sense to use lazy
loading for the list.

Signed-off-by: Jarno Elovirta <jarno@elovirta.com>
Signed-off-by: Jarno Elovirta <jarno@elovirta.com>
@robander
Copy link
Member Author

robander commented Aug 6, 2018

Added support for listing trantypes from CLI, IMO that's in the same class or requirement as listing plugins. @robander are you ok with this addition?

👍 yes, was hoping we'd get that included as well.

Signed-off-by: Jarno Elovirta <jarno@elovirta.com>
@jelovirt jelovirt moved this from In progress to Review in 3.2 Aug 9, 2018
@robander robander merged commit e4226ef into dita-ot:develop Aug 9, 2018
3.2 automation moved this from Review to Done Aug 9, 2018
@robander robander added this to the 3.2 milestone Aug 9, 2018
infotexture added a commit to dita-ot/docs that referenced this pull request Oct 19, 2018
Implemented in the 3.2 release cycle:

- dita-ot/dita-ot#1960
- dita-ot/dita-ot#3037

Signed-off-by: Roger Sheen <roger@infotexture.net>
Copy link
Member

@infotexture infotexture left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the plug-ins are listed alphabetically, whereas transtypes seem randomly ordered.

Would it be possible to align this and list transtypes in alphabetical order too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Command-line interface issues feature New feature or request
Projects
No open projects
3.2
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants