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

feat(verifier): allow simplifying graph output data #5286

Merged
merged 1 commit into from May 24, 2022

Conversation

shahms
Copy link
Contributor

@shahms shahms commented May 24, 2022

This adds two additional verifier flags which control removing the /kythe/(edge/)? prefix and hiding node VNames when dumping graphviz graphs. This results in much more compact and readable graphs (to say nothing of the dot file itself).

This also incidentally updates the PrinterPrinter interface to use absl::string_view instead of const std::string&

@shahms shahms requested review from zrlk and a team May 24, 2022 18:23
@jaysachs
Copy link
Contributor

Thanks for this.

One other thing I was thinking would be nice is to auto-generate labels where they don't exist, based on the node kind, e.g. function123, record124, etc.

@shahms shahms merged commit b655498 into kythe:master May 24, 2022
@shahms shahms deleted the verifier-minimal-graphs branch May 24, 2022 20:46
@@ -53,6 +53,10 @@ ABSL_FLAG(std::string, goal_regex, "",
ABSL_FLAG(bool, convert_marked_source, false,
"Convert MarkedSource-valued facts to subgraphs.");
ABSL_FLAG(bool, show_anchors, false, "Show anchor locations instead of @s");
ABSL_FLAG(bool, show_vnames, true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if these flag names (as well as other graphviz-related flags) ought to have a dot_ or graphviz_ (or gv_) prefix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly? In theory they could apply to the JSON dumps as well (but don't currently).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants