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

Sort options in help output by key to assist user navigation #21

Merged
merged 1 commit into from Apr 4, 2019

Conversation

romeara
Copy link
Member

@romeara romeara commented Apr 3, 2019

Description

This updates help content to sort the documented properties by key, to assist with navigation. This will naturally group like properties with similar prefixes.

@romeara
Copy link
Member Author

romeara commented Apr 3, 2019

Note - Eclipse is fighting me about running this locally, so I could use some assistance verifying the desirability of the resulting output

@taikuukaits
Copy link
Contributor

I do agree we should sort better by default, so this is a good idea for an improvement.

That said, I do have a few concerns:

  1. Should we sort by group and then by property key?
  2. If not, should we also modify help verbose and have that sort by key?
  3. I don't believe that is the proper place to sort, it should be in the method before they are printed. For example 'printHelpFilteredByPrintGroup' does it's own sort. Maybe printOptions can take a sort operation which would make each method have to decide how it wants to sort.
  4. Also currently properties of different groups are separated by a new-line. If we simply sort by key we will have a lot of additional newlines. So should we remove the group separated new line in this case? It's possible we want to have the method also control if groups are separated by new line.

@stevebillings
Copy link
Contributor

There is a request (IDETECT-1234) for arranging by group, currently scheduled for 5.5.0. I think that's a reason to sort first by group.

@romeara
Copy link
Member Author

romeara commented Apr 4, 2019

The by group sounds good, but I'm hitting an issue looking to implement it - where can I get the group of an option? It seems to only be in an annotation on the enum, which I can't get and sort by without a bunch of reflection nonsense. Am I missing something?

@taikuukaits
Copy link
Contributor

I think lines 76-82 do exactly what you want, you just want to apply them to every help print. It sorts first by group, then by key.

options.stream().sorted((o1, o2) -> {
            if (o1.getDetectOptionHelp().primaryGroup.equals(o2.getDetectOptionHelp().primaryGroup)) {
                return o1.getDetectProperty().getPropertyKey().compareTo(o2.getDetectProperty().getPropertyKey());
            } else {
                return o1.getDetectOptionHelp().primaryGroup.compareTo(o2.getDetectOptionHelp().primaryGroup);
            }
        })

@taikuukaits taikuukaits merged commit 56bc118 into master Apr 4, 2019
@taikuukaits taikuukaits deleted the doc/romeara/sort-options branch April 4, 2019 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants