Skip to content

Doc: add and fix docs for summary report subject line #148

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

PartialShawn
Copy link
Contributor

I added the subject line to the cli help response, and updated the code doc to use = for the parameter, and fixed where the quoting was.

The example uses unescaped spaces, which works fine for me. But the code says spaces must be escaped. I didn't fix this inconsistency, as you may want to go in a direction where one or the other is required. But as it seems to work with spaces, I'd recommend leaving it as allowing unescaped spaces.

Thanks for implimenting the subject line feature.

@PartialShawn
Copy link
Contributor Author

Wait, I didn't check my last test. I'm not actually sure how to escape spaces anyway, so I guess the spaces comment should be removed?

@liuch
Copy link
Owner

liuch commented Nov 25, 2024

As for the colon - you are right, it is my bad. Paired quotes can be used both ways, your version looks better.

@liuch
Copy link
Owner

liuch commented Nov 25, 2024

Wait, I didn't check my last test. I'm not actually sure how to escape spaces anyway, so I guess the spaces comment should be removed?

As you see fit. You could create a new PR or add another commit.

@liuch
Copy link
Owner

liuch commented Nov 25, 2024

The example uses unescaped spaces, which works fine for me. But the code says spaces must be escaped. I didn't fix this inconsistency, as you may want to go in a direction where one or the other is required. But as it seems to work with spaces, I'd recommend leaving it as allowing unescaped spaces.

I was sloppy and made an unfortunate point. I just meant that if there are spaces in the subject, you should either use quotation marks or escape them. The following uses are possible:

  • subject=My\ cool\ report
  • "subject=My cool report"
  • subject="My cool report"

But this use is incorrect: subject=My cool report

@@ -40,7 +40,7 @@
* when the `domain` options is set to `all`. Only makes sense if the user_management mode is active.
* The default value is `admin`.
* The `subject` parameter is optional. It allows you to specify the subject line of the summary report message
* instead of the default subject line. Remember to escape spaces and special characters!
* instead of the default subject line. Remember to escape spaces and special characters using HTML entities!
Copy link
Owner

Choose a reason for hiding this comment

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

Special HTML characters do not need to be encoded. Only special shell characters need to be escaped to ensure that the parameters are passed without distortion. The script will do the rest. That might be worth clarifying.

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.

2 participants