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

Replace AsciiDoc headings with new recommended style #17

Open
mbrukman opened this issue Dec 22, 2017 · 3 comments · May be fixed by #18
Open

Replace AsciiDoc headings with new recommended style #17

mbrukman opened this issue Dec 22, 2017 · 3 comments · May be fixed by #18

Comments

@mbrukman
Copy link
Contributor

In issue #16, we want to replace "Janus Graph" with "JanusGraph", which shortens the headings by a single character. However, the AsciiDoc headings currently in use have the same # of characters are the headings, e.g.,

Heading
~~~~~~~

That said, this is a deprecated heading, and was fixed in JanusGraph in this commit which also says:

As described in the Asciidoc best practices document:
http://asciidoctor.org/docs/asciidoc-recommended-practices/#section-titles
using heading markers using underlines of various characters is not recommended
for several reasons:

  • hard to remember which character means which level header
  • hard to maintain the correct number of characters (must match heading line)

That commit also includes a complete Python script for making this update for AsciiDoc files, which will be used to convert the headings to the new recommended format, and make it easier to cleanup the name format.

mbrukman added a commit to mbrukman/graph-book that referenced this issue Dec 22, 2017
Fixes krlawrence#17.

As described in the Asciidoc best practices document:
http://asciidoctor.org/docs/asciidoc-recommended-practices/#section-titles
using heading markers using underlines of various characters is not recommended
for several reasons:

* hard to remember which character means which level header
* hard to maintain the correct number of characters (must match heading line)

This change takes the same approach as a similar change in JanusGraph:
JanusGraph/janusgraph@dc67ce7
with some minor modifications to handle Windows-style EOL `\r\n` in this file,
whereas the other change works only with UNIX-style EOL `\n`.

What that required was stripping just the Windows-style EOL characters before doing
the regex matches:

```
line = line.rstrip('\r\n')
```

Note that using `line = line.rstrip()` doesn't work here, because some lines
already end with a whitespace character, which leads to spurious diffs.

Then, we manually added them back via:

```
sys.stdout.write('%s\r\n' % ...)
```

Using `print` in this case would give us UNIX-style EOL, causing the entire file
to show a diff on every line.

Here's the entire file with modifications for this run, which may be useful in
the future if/when someone needs to reapply this to an AsciiDoc file formatted
with Windows EOL chars:

```
#!/usr/bin/python

import re
import sys

def main(argv):
    # http://asciidoctor.org/docs/asciidoc-recommended-practices/#section-titles
    patterns = [
        (re.compile('^=+$'), '='),
        (re.compile('^-+$'), '=='),
        (re.compile('^~+$'), '==='),
        (re.compile('^\^+$'), '===='),
        (re.compile('^\++$'), '====='),
    ]

    with open(argv[1], 'r') as input_file:
        prev_line = None
        curr_line = None
        for line in input_file.readlines():
            line = line.rstrip('\r\n')
            prev_line = curr_line
            curr_line = line

            if prev_line is None:
                continue

            for pattern, heading in patterns:
                if pattern.match(curr_line) and len(prev_line) == len(curr_line):
                    sys.stdout.write('%s %s\r\n' % (heading, prev_line))
                    prev_line = None
                    curr_line = None
                    break

            if prev_line is not None:
                sys.stdout.write('%s\r\n' % prev_line)

        # end for
        if curr_line is not None:
            sys.stdout.write(curr_line)

    # end with

if __name__ == '__main__':
    main(sys.argv)

```
@mbrukman mbrukman linked a pull request Dec 22, 2017 that will close this issue
@krlawrence
Copy link
Owner

I would like to hold off just a little while on doing this one as I really like how the old style headings look in Vim when I am editing the manuscript. I plan to do a lot of editing over the next few days. After that would be a good time for me to also change the heading style. I also note that the Asciidoctor guide that you referenced is listed as a draft and subject to change but I agree the point about keeping the line length the same does become a big pain. I'll get to this one after I get some more edits done. I just did a fairly big edit on Section 3 this morning.

@mbrukman
Copy link
Contributor Author

Makes sense, I'm happy to rerun the script whenever you're ready — just ping either this issue or PR #18 to let me know.

@krlawrence krlawrence added this to High priority in Planning Aug 29, 2020
@krlawrence krlawrence moved this from High priority to Needs triage in Planning Aug 30, 2020
@krlawrence krlawrence moved this from Needs triage to Low priority in Planning Aug 30, 2020
@krlawrence
Copy link
Owner

Let's revisit this for the Second Edition on which work is just getting started

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Planning
  
Low priority
Development

Successfully merging a pull request may close this issue.

2 participants