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

Revise log messages for #3973 #4287

Merged
merged 52 commits into from Jan 7, 2024
Merged

Conversation

infotexture
Copy link
Member

@infotexture infotexture commented Sep 29, 2023

Description

Error message refactoring for #3973

  • Add blank lines between messages for readability (8dfb8ca)
  • Normalize whitespace (d6369fc)
  • End all reason elements with periods (cf6c169)
  • Sort messages by ID within files (067e5b7)
  • Use single quotes for % placeholders (a17c7c8)
  • Mark unused message duplicates as deprecated (f097570)
  • Use XML-domain–style markup for attribute & element references
    (@rev attribute, <topicref> element, etc.)
    (9d64912, 4253c33 , 13e57f6, and 62c8d0b)
  • Align references to DITA and DITAVAL files (479930e)
  • Replace "tag" references with "element" (e060b7a)
  • Update “file” references to “resource” (0c7753f)
  • Align “couldn’t” & “could not” → “cannot” (adb9a2d)
  • Deprecate any obsolete messages that are no longer used in the code

@infotexture infotexture added the enhancement Changes to an existing feature label Sep 29, 2023
@chrispy-snps
Copy link
Contributor

@infotexture, I appreciate this work! It adds polish to the software and makes a good first impression on new users.

@infotexture
Copy link
Member Author

@jelovirt @robander ❓ Any preference on how to format empty response elements?

The vast majority are currently expanded as <response></response> but there are also several collapsed to <response/>.

➕/➖ We could save a few bytes by collapsing them all, but it would cause more diffs.

That said, by the time this PR is ready, we may end up touching most lines anyway, so I'm not sure the extra diffs would be all that disruptive.

@infotexture infotexture force-pushed the feature/3973-log-messages branch 2 times, most recently from cf6c169 to 067e5b7 Compare October 4, 2023 21:46
infotexture added a commit that referenced this pull request Oct 4, 2023
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
infotexture added a commit that referenced this pull request Oct 29, 2023
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
@chrispy-snps
Copy link
Contributor

@infotexture - do you want me to search for obsolete messages? I can script something up for this.

@infotexture
Copy link
Member Author

@chrispy-snps Sure, if you have an easy way of finding those automatically, that would help. 🙏

We may not remove them yet, but could mark them as deprecated/unused.

infotexture added a commit that referenced this pull request Oct 29, 2023
Per #4287 (comment)

Most are expanded, so this aligns those that were collapsed.

Signed-off-by: Roger Sheen <roger@infotexture.net>
@chrispy-snps
Copy link
Contributor

chrispy-snps commented Oct 29, 2023

@infotexture - here's a Perl script that you can run from the root of the dita-ot development repo:

find_unused_messages.zip

The script does the following:

  1. Makes a list of all message IDs defined in .xml files
  2. Removes all message IDs referenced in .java and .xsl files
  3. Prints all the remaining (unused) message IDs
    • Each ID is accompanied by the file(s) from step 1 where it is defined

For conservatism, step 1 considers messages defined in commented code (so we can delete them even if commented), but step 2 does not consider references in commented code.

But I might be missing something because it's claiming to find quite a number of unused messages. Can you run this on your side and see what you think?

@infotexture
Copy link
Member Author

infotexture commented Nov 7, 2023

@chrispy-snps The script returns 139 supposedly unused message IDs, but the main messages_template.xml file only contains ~160 messages (plus 20-odd more in default plug-ins), so I'm skeptical that that many are really unused. 🧐

My gut feeling says ~80% can't be obsolete. 🤔

If it were ~20% it would seem more plausible, but my Perl fu isn't strong enough to review the logic in the script.

@jelovirt @robander Any thoughts?

Results
Unused message IDs:
DOTA002F (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTA003F (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTA004F (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTA006W (./src/main/config/messages.xml ./src/main/plugins/org.dita.xhtml/resource/messages.xml ./src/test/resources/messages.xml)
DOTA007E (./src/main/config/messages.xml ./src/main/plugins/org.dita.xhtml/resource/messages.xml ./src/test/resources/messages.xml)
DOTA008E (./src/main/config/messages.xml ./src/main/plugins/org.dita.xhtml/resource/messages.xml ./src/test/resources/messages.xml)
DOTA009E (./src/main/config/messages.xml ./src/main/plugins/org.dita.xhtml/resource/messages.xml ./src/test/resources/messages.xml)
DOTA010E (./src/test/resources/messages.xml)
DOTA011W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTA012W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTA013F (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTA015F (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTA066F (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
DOTA069W (./src/main/config/messages.xml ./src/main/plugins/org.dita.xhtml/resource/messages.xml ./src/test/resources/messages.xml)
DOTJ001F (./src/test/resources/messages.xml)
DOTJ002F (./src/test/resources/messages.xml)
DOTJ003F (./src/test/resources/messages.xml)
DOTJ004F (./src/test/resources/messages.xml)
DOTJ006F (./src/test/resources/messages.xml)
DOTJ007E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ007W (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTJ009E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ010E (./src/test/resources/messages.xml)
DOTJ011E (./src/test/resources/messages.xml)
DOTJ015F (./src/test/resources/messages.xml)
DOTJ016F (./src/test/resources/messages.xml)
DOTJ017F (./src/test/resources/messages.xml)
DOTJ018I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ019E (./src/test/resources/messages.xml)
DOTJ021W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ023E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ024W (./src/test/resources/messages.xml)
DOTJ026E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ027W (./src/test/resources/messages.xml)
DOTJ028E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ029I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ032E (./src/test/resources/messages.xml)
DOTJ038E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ045W (./src/test/resources/messages.xml)
DOTJ050W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ051E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ053W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTJ063E (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTJ072E (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTJ073E (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX001W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX002W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX003I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX004I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX005E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX006E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX007I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX008W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX009W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX010E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX011W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX012W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX013E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX014E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX015E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX016W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX017E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX018I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX019W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX020E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX021E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX022W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX023W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX024E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX025E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX026W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX027W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX028E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX029I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX030W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX031E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX032E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX033E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX034E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX035E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX036E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX038I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX039W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX040I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX041W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX042I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX043I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX044E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX045W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX046W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX047W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX048I (./src/main/config/messages.xml ./src/main/plugins/org.dita.htmlhelp/resource/messages.xml ./src/test/resources/messages.xml)
DOTX049I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX050W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX051W (./src/test/resources/messages.xml)
DOTX052W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX053E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX054W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX055W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX056W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX057W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX058W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX059I (./src/test/resources/messages.xml)
DOTX060W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX061W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX062I (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX063W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX066W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX067E (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX068W (./src/main/config/messages.xml ./src/main/config/messages_template.xml ./src/test/resources/messages.xml)
DOTX069W (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX070W (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX071E (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX071W (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX072I (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX073I (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX074W (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX075W (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX076E (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
DOTX077I (./src/main/config/messages.xml ./src/main/config/messages_template.xml)
INDX001I (./src/main/config/messages.xml ./src/main/plugins/org.dita.index/messages.xml)
INDX002E (./src/main/config/messages.xml ./src/main/plugins/org.dita.index/messages.xml)
INDX003E (./src/main/config/messages.xml ./src/main/plugins/org.dita.index/messages.xml)
PDFX001W (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX002W (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX003W (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX004F (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX005F (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX006E (./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX007W (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX008W (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX009E (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX010W (./src/test/resources/messages.xml)
PDFX011E (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml ./src/test/resources/messages.xml)
PDFX012E (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml)
PDFX013F (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2/resource/messages.xml)
XEPJ001W (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2.xep/resource/messages.xml)
XEPJ002E (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2.xep/resource/messages.xml)
XEPJ003E (./src/main/config/messages.xml ./src/main/plugins/org.dita.pdf2.xep/resource/messages.xml)

@infotexture
Copy link
Member Author

— Just a thought:

Could it be that the code composes certain message IDs as prefix+number+suffix, but the script is looking for the ID verbatim (like DOTA002F), so it comes up empty even if the message is referenced (composed) in the code? 🤔

@chrispy-snps
Copy link
Contributor

@infotexture - I wondered the same thing, but I could not find any code instances of messages being constructed dynamically like that.

The lines in your "results" excerpt seem to have run together. Can you reformat them?

I am as perplexed by the results as you are. Hopefully I did something wrong, but if so, I haven't been able to figure it out.

@infotexture
Copy link
Member Author

The lines in your "results" excerpt seem to have run together. Can you reformat them?

Sorry, that broke when I deleted an empty line between the <summary> element and the code block fence ``` .

✅ Fiddly formatting fixed.

@infotexture infotexture linked an issue Nov 9, 2023 that may be closed by this pull request
@infotexture infotexture marked this pull request as ready for review December 7, 2023 15:35
@raducoravu
Copy link
Member

@infotexture great job, I skimmed a bit through the updated messages.

Looking in the messages templates, there are still places like this where double quotes are still used instead of single quotes like:

 and set the value to "warn" or "quiet"
 attribute value "%1"

Maybe replace "should" with "must" here:

The value should contain '#'

Here maybe remove the last " href=""":

Found a link or cross reference with an empty @href attribute: href=""

Maybe rephrase messages which contain "will" to avoid future tenses.

@kirkilj
Copy link

kirkilj commented Dec 8, 2023

This might be as good a place as any to mention that I tried to create a god-awful Python script with a god-awful regex parse_ot_log.py.txt to parse OT error messages into distinct fields. My latest attempt tried to parse the messages.xml file and use it to compute a regex pattern to parse a log's actual messages. It's a work in progress, to say the least.

In some messages no file paths are mentioned, while in others there are 1 or possibly two. Has there been any discussion of also providing an XML version of an OT log so we don't have to infer structure from raw text that already has a structure to begin with? Is an OT-log schema out of the question to pursue separately? My intention is to save a structured representation of these logs in our CI/CD environment so our Information Architecture group can ingest them into a datastore can do analytics on them, if not some ML experiments at some point.

I can move this into a discussion if anyone is interested in engaging with it there.

infotexture added a commit that referenced this pull request Dec 20, 2023
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
infotexture added a commit that referenced this pull request Dec 20, 2023
Per #4287 (comment)

Most are expanded, so this aligns those that were collapsed.

Signed-off-by: Roger Sheen <roger@infotexture.net>
Signed-off-by: Roger Sheen <roger@infotexture.net>
Signed-off-by: Roger Sheen <roger@infotexture.net>
Signed-off-by: Roger Sheen <roger@infotexture.net>
Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per
- #4287 (comment)
- #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Per #4287 (comment)

Signed-off-by: Roger Sheen <roger@infotexture.net>
Copy link
Member

@robander robander left a comment

Choose a reason for hiding this comment

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

Really big cleanup project, looks ready to go...

@chrispy-snps
Copy link
Contributor

@infotexture - as someone with a technical writing background, this is an impressive piece of work. Thanks for doing it.

@infotexture infotexture merged commit 1d75a16 into develop Jan 7, 2024
4 checks passed
@infotexture infotexture deleted the feature/3973-log-messages branch January 7, 2024 15:00
@infotexture infotexture added this to the Next milestone Jan 24, 2024
@infotexture infotexture self-assigned this Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Changes to an existing feature
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Change log message copy to use consistent language
6 participants