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

find "Abstract" and make it a link target "#abstract" (fixes #24) #25

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

reschke
Copy link

@reschke reschke commented Aug 16, 2022

No description provided.

Copy link
Collaborator

@larseggert larseggert left a comment

Choose a reason for hiding this comment

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

The syntax needs to be fixed to avoid deprecation warning. See how similar regexes now look on main.

Also, might want to handle these sections as well, for parity with what the HTML format has:

  • About This Document
  • Note to the RFC Editor
  • Status of This Memo
  • Copyright Notice
  • Table of Contents

@reschke
Copy link
Author

reschke commented Sep 27, 2022

Is the deprecation related to not using the Raw String Notation? In which case it's fixed now.

Regarding the other sections; I'll check. They may have no explicit anchor assigned by xml2rfc (except for the implied "n-...", which I believe we should not rely on). So this might require changes in xml2rfc as well.

@larseggert
Copy link
Collaborator

Datatracker says:

/home/dev/.local/lib/python3.9/site-packages/rfc2html.py:282: DeprecationWarning: invalid escape sequence \g  text = re.sub(r"(?m)^(\s*)(Abstract)$", """\g<1><a id="abstract" href="#abstract" class="selflink">\g<2></a>""", text, count=1)
/home/dev/.local/lib/python3.9/site-packages/rfc2html.py:283: DeprecationWarning: invalid escape sequence \g
  text = re.sub(r"(?m)^(\s*)(Table of Contents)$", """\g<1><a id="table-of-contents" href="#table-of-contents" class="selflink">\g<2></a>""", text, count=1)

@larseggert
Copy link
Collaborator

You also need to wrap this in h2 if you want it to be picked up by the table of contents generator.

@reschke
Copy link
Author

reschke commented Sep 28, 2022

FWIW, the whole code uses the deprecated notation. The changes are consistent with the existing code,

Wouldn't it make sense to separate these kinds of changes into separate tickets, and to apply the changes to the complete codebase?

@reschke
Copy link
Author

reschke commented Sep 28, 2022

You also need to wrap this in h2 if you want it to be picked up by the table of contents generator.

That may make sense, but then this should be done uniformly through the whole codebase, no?

@reschke
Copy link
Author

reschke commented Sep 28, 2022

The syntax needs to be fixed to avoid deprecation warning. See how similar regexes now look on main.

Also, might want to handle these sections as well, for parity with what the HTML format has:

* About This Document

* Note to the RFC Editor

* Status of This Memo

* Copyright Notice

* Table of Contents

Agreed. Let's see what xml2rfc currently generates:

apparently hard-wired and could be relied on

  • "Abstract" - "#abstract"

under author's control

  • "Note to the RFC Editor": this is a standard <note> where the author controls the anchor; there are no heuristics to find out what the author used - also irrelevant for RFCs

getting auto-generated anchors based on text contents

  • "About This Document": "#name-about-this-document" (auto-generation for identifiers that have not been specified otherwise) - also irrelevant for RFCs
  • "Status of This Memo" - "#name-status-of-this-memo" (auto-generation for identifiers that have not been specified otherwise)
  • "Copyright Notice" - "#name-status-of-this-memo" (auto-generation for identifiers that have not been specified otherwise)
  • "Table of Contents" - "#name-table-of-contents" (auto-generation for identifiers that have not been specified otherwise)
  • "Index" - "#name-index" (auto-generation for identifiers that have not been specified otherwise)
  • "Author's Addresses" - "#name-authors-addresses" (auto-generation for identifiers that have not been specified otherwise)

Proposal

Stick with the change for "abstract" for now. Raise issues for xml2rfc to generate stable anchors for the other sections, and once they are agreed upon, generate them here as well.

@larseggert
Copy link
Collaborator

FWIW, the whole code uses the deprecated notation. The changes are consistent with the existing code,

Are you sure you are up to date with main?

rfc2html.py Outdated Show resolved Hide resolved
@reschke
Copy link
Author

reschke commented Sep 28, 2022

FWIW, the whole code uses the deprecated notation. The changes are consistent with the existing code,

Are you sure you are up to date with main?

Likely not. Will check.

Copy link
Collaborator

@larseggert larseggert left a comment

Choose a reason for hiding this comment

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

LGTM but the RPC should check whether this works for them as well.

@rjsparks
Copy link
Member

@reschke - just rediscovered this request - it fell through the cracks, apologies. will vet with the RPC.

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.

3 participants