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

Convert HTTP to Markdown #7853

Merged
merged 9 commits into from
Aug 13, 2021
Merged

Convert HTTP to Markdown #7853

merged 9 commits into from
Aug 13, 2021

Conversation

teoli2003
Copy link
Contributor

This is the final PR that fixes #7536.

Command used:
yarn md h2m web/http --locale en-US --mode replace

Then I made two commits. First the rename, then the modifications. Don't squash merge this!

Conversion report available at: gist

Only the tables with a class properties are not converted. There are no other problems reported by the tool.

@teoli2003 teoli2003 requested a review from a team as a code owner August 12, 2021 10:36
@github-actions

This comment has been minimized.

@sideshowbarker
Copy link
Collaborator

The first paragraph of files/en-us/web/http/headers/content-security-policy/navigate-to/index.md seems to have failed conversion.

The HTTP {{HTTPHeader("Content-Security-Policy")}} (CSP)
**`navigate`\*\***`-to`** directive
restricts the URLs to which a document can initiate navigations by any means including
{{HTMLElement("form")}} (if {{CSP("form-action")}} is not specified),
{{HTMLElement("a")}}, {{DOMxRef("window.location")}}, {{DOMxRef("window.open")}}, etc.
This is an enforcement on what navigations this document initiates **not\*\*
on what this document is allowed to navigate to.

Can kind of see what from Looking at the HTML source:

<p>The HTTP {{HTTPHeader("Content-Security-Policy")}} (CSP)
  <code><strong>navigate</strong></code><strong><code>-to</code></strong> directive
  restricts the URLs to which a document can initiate navigations by any means including
  {{HTMLElement("form")}} (if {{CSP("form-action")}} is not specified),
  {{HTMLElement("a")}}, {{DOMxRef("window.location")}}, {{DOMxRef("window.open")}}, etc.
  This is an enforcement on what navigations this document initiates <strong>not</strong>
  on what this document is allowed to navigate to.</p>

Anyway, I pushed a fix to the branch.

@Ryuno-Ki
Copy link
Collaborator

Heads up, the button currently show „squash and merge“ to me.
Is automerge (via rebase) possible to set already in this repo?

@sideshowbarker
Copy link
Collaborator

General comment: 144 out of the 297 — almost 50% — of the markdown files in this PR have HTML tables that failed conversion. That seems like relatively a lot — so I wonder if maybe there’s some automated way we could pre-process the sources to prevent the table conversion from failing.

@sideshowbarker
Copy link
Collaborator

I pushed a series of commits for some other issues I found when randomly checking a sample of files from the PR, and from running markdownlint.


The response is extremely simple too: it only consisted of the file itself.

<HTML>
Copy link
Collaborator

@hamishwillee hamishwillee Aug 13, 2021

Choose a reason for hiding this comment

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

Even though the indentation works as a method of indicating pre/code blocks, I see using the backticks as a much "safer" markup.

It looks like the converter does it this way for everything that is a plain pre in html. Suggest we ask for that to be changed. Reasons

  • very easy to stuff up up when editing - forget to indent one thing
  • Both yari and github support markup if you specify the codeblock type as http. If we add the backticks to start with, remembering to add http later is easy.

So this is what it looks like if you specify the type

    GET /mypage.html HTTP/1.0
    User-Agent: NCSA_Mosaic/2.0 (Windows 3.1)

    200 OK
    Date: Tue, 15 Nov 1994 08:12:31 GMT
    Server: CERN/3.0 libwww/2.17
    Content-Type: text/html
    <HTML>
    A page with an image
      <IMG SRC="/myimage.gif">
    </HTML>

@hamishwillee
Copy link
Collaborator

General comment: 144 out of the 297 — almost 50% — of the markdown files in this PR have HTML tables that failed conversion. That seems like relatively a lot — so I wonder if maybe there’s some automated way we could pre-process the sources to prevent the table conversion from failing.

@sideshowbarker That is probably the table at the top of every header

Header type {{Glossary("Response header")}}
{{Glossary("Forbidden header name")}} no
{{Glossary("CORS-safelisted response header")}} no

Similar to the discussion with @wbamberg on CSS, do you think we might autogenerate these from frontmatter?

@sideshowbarker
Copy link
Collaborator

Similar to the discussion with @wbamberg on CSS, do you think we might autogenerate these from frontmatter?

Yeah that would certainly seem do-able without a huge level of effort. But that said, I guess it doesn’t need to block this PR — could be addressed in a follow-up

@teoli2003
Copy link
Contributor Author

teoli2003 commented Aug 13, 2021

Thanks all for the thorough feedback!

So here are the next steps I plan to perform:

  • Open an issue on mdn/content to get the properties tables fixed in the mid-term. This will keep the discussion rolling past this PR.
  • Add a commit here, manually fixing the case of <pre> blocks translated to Markdown without backticks (that is, adding the backticks). Thanks, @hamishwillee for detecting this!
  • Open an issue on yari to get the converter updated for that case.
  • Request a final round of review here, then rebase and merge this PR before it rots
    (@hamishwillee: PR FF92 AVIF: Remove experimental feature and update accept headers. #7873 😉)
  • Open an issue to get our MD meta-docs slightly extended to hint about markdownlint with the config file suppressing MD040 and long-line errors)

@sideshowbarker: thanks a lot for the thorough review. I didn't know about markdownlint; it is now installed and integrated with my editor 👍 .
@Ryuno-Ki: I don't know anything about auto-merge, but we want to do a rebase and merge here.

We are close!

@Ryuno-Ki
Copy link
Collaborator

@teoli2003
Copy link
Contributor Author

I fixed the triple ticks for <pre> tags.

I think we are good to go! Please have a last look; I will open the promised discussions/follow-up later today, then rebase and merge.

@teoli2003
Copy link
Contributor Author

Alea jacta est.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert Web/HTTP to markdown
4 participants