Skip to content

Conversation

@mathew2103
Copy link

@mathew2103 mathew2103 commented Oct 8, 2025

Fixes #77

@mathew2103

This comment was marked as duplicate.

@mathew2103
Copy link
Author

Example outputs:
image
image

Copy link
Owner

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

Comment on lines 83 to 91
// Error message
if (data.message)
return `${digCmd}\n${data.message}`;

// No results
if (!data.answer.length)
return `${digCmd}\nNo records found${data.flags.cd
? `\n\n${DNSSEC_DISABLED_WARNING_MESSAGE}`
: ''}`;
Copy link
Owner

Choose a reason for hiding this comment

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

Do these two early returns need to handle comments?

return `${output(finalRows)}${data.flags.cd
? `\n${DNSSEC_DISABLED_WARNING_MESSAGE}`
: ''}`;
: ''}\n${Array.isArray(data.comment) ? `**Comments:** ${data.comment.join("; ")}` : (data.comment ? `**Comments:** ${data.comment}` : "")}`;
Copy link
Owner

Choose a reason for hiding this comment

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

👀 Two thoughts here:

  1. Instead of the current formatting you have for comments, can we make them formatted as a quote perhaps?
  2. The maxLength / finalRows logic doesn't account for the length of the comments you're appending here.

type,
`@${provider.dig}`,
'+noall',
'+answer',
Copy link
Owner

Choose a reason for hiding this comment

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

I think we also need +comments in the equivalent dig command now?

`@${provider.dig}`,
'+noall',
'+answer',
options.short ? '+short' : null,
Copy link
Owner

Choose a reason for hiding this comment

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

In the spirit of the +short option from dig itself, I wonder if we shouldn't show the comments when short is requested?

@MattIPv4
Copy link
Owner

@mathew2103 any update on this?

@mathew2103
Copy link
Author

Got a bit tied up with uni exams, I will make changes as per the review this week.. Apologies

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.

Expose comments in dig result embed

2 participants