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 CommentOutputHook #650

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

raviks789
Copy link
Contributor

@raviks789 raviks789 commented Sep 22, 2022

CommentOutputHook gets the transformation for the comments. For example creating ticket links in the comments using icinga web generic tts module.

@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Sep 22, 2022
Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

Comparing this with the PluginOutputHook, the latter also gets two additional parameters passed: $commandName and $enrichOutput.

I think we should pass similar parameters here. There's no command name of course, but an author's name.

Also, $enrichOutput is meant to control whether HTML is processed/generated or not. A similar thing is possible here since Markdown is supported. Though, we always render Markdown, just not all types of it. Lists for example only render inline elements. Block elements are only rendered in the detail views. This differentiation should also be possible for hooks, so let's pass them the $viewType. ('list' or 'detail', use constants for these, don't just pass hardcoded strings)

@nilmerg
Copy link
Member

nilmerg commented Oct 5, 2022

Said all this, we're not gonna merge this anytime soon. (But please finish this as I described above) I've talked about this with @lippserd and we agreed to postpone this to next year Q2. The reason for this is that we're not sure yet if the interface should be kept this way or if it should allow passing the comment model as well. This may allow implementors to retrieve more meta data (such as groups and custom variables) to generate more sophisticated details.

@raviks789 raviks789 force-pushed the feature/add-commmentoutput-hook branch from ec0fa06 to a8993eb Compare October 5, 2022 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants