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

fix(build/parser): parse empty string args for macros as Nones #88

Merged
merged 7 commits into from
Jan 22, 2025

Conversation

yin1999
Copy link
Member

@yin1999 yin1999 commented Jan 18, 2025

Description

replace empty string args for macos with Nones

Motivation

In mdn/content, we are used to use empty string to represent the argument is not provided, which should be None instead of Some(EmptyString). But we have use some judgement like if let Some(anchor) = anchor to check if the string argument is provided. This will evaluate to true as the judgement encounters a string.

I found an incorrect case in /en-US/docs/Mozilla/Add-ons/WebExtensions/API/pageAction/onClicked page:

image

There is a hash symbol at the end of the link text.

I think the easiest solution is to replace the empty string args with Nones. As we use similar logic to the above to determine whether the parameters are provided in many macos. Some cases (Maybe not all of them):

  • crates/rari-doc/src/templ/templs/links/domxref.rs (we have checked whether the arg is an empty string)
  • crates/rari-doc/src/templ/templs/links/htmlxref.rs
  • crates/rari-doc/src/templ/templs/links/http.rs
  • crates/rari-doc/src/templ/templs/links/jsxref.rs
  • crates/rari-doc/src/templ/templs/links/webextapixref.rs (the one I found the issue)

I haven't fully tested this change, so I'm not sure if it will break any pages. If you have any ideas, please let me know.

@yin1999 yin1999 changed the title fix(build/parser): replace emptry string args for macos with Nones fix(build/parser): replace empty string args for macos with Nones Jan 19, 2025
@fiji-flo
Copy link
Contributor

Thanks for this. I opened a PR yin1999#1 to move the logic into the actual parser. Please take a look and merge.

tokenize empty string and treat as none
@fiji-flo fiji-flo changed the title fix(build/parser): replace empty string args for macos with Nones fix(build/parser): parse empty string args for macros as Nones Jan 22, 2025
@fiji-flo fiji-flo merged commit 4f5751f into mdn:main Jan 22, 2025
1 check passed
@yin1999 yin1999 deleted the fix-replace-emptry-string-with-none branch January 22, 2025 08:32
This was referenced Jan 22, 2025
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