Process TracWiki links and formatting. (#4) #6

danuker · 2021-01-26T12:39:41Z

Scope

There are TracWiki markup links which need to be converted. Here are examples of files (taken from #2 (comment)_ ):

TracWiki markup - https://trac.chevah.com/wiki/WikiStart?action=edit
RST markup with Trac directives - https://trac.chevah.com/wiki/Development/Python?action=edit
Mixed of RST with TracWiki macros (see bottom) - https://trac.chevah.com/wiki/Administrative?action=edit

At first, we can use this ticket to convert RST Trac links directives.

Later we should look at converting whole TracWiki pages.

Changes

TracWiki links should now work. I have added a new match case to _trac_rst_wiki_to_github_links, as well as new methods that handle TracWiki link syntax (with or without link text different from the article).

Also Trac tickets should work. There were only 3 of them, but it was easy enough to automate.

Only one malformed :trac: reference remains, which will be addressed manually.

TracWiki syntax: the script handles the following:

title/heading syntax: heading (===) and subheading (---)
lists: it unindents single-space-quoted lists, and adds an empty line before the first list element. The majority of files looks good; but nesting conversion is not proper.

What is not done

About lists: I did not properly implement nesting. I started to, but it turned out to be too difficult for me, considering the diminishing returns. An exhaustive (as of 2021-01-27) list of failures:

Architecture/Agent/General section "Remote file", under "steps"
Development/GetToKnow section "Products documentation and download pages".
Infrastructure/OutOfOrderMachines Mosna under "network cards"
Infrastructure/Services/VPN under "Overton"
Requirements/Vision, last two list items on page
Support/General section "Initial Assessment"
Support/CustomerDataBase/Template.rst and some files created from that template.

In addition, {{{ }}} code blocks need to be converted manually, by searching for {{{ in a commit before stripping the {{{#!rst }}} armor.

How to try and test the changes

reviewers: @adiroiban

First, copy the config.py.sample to config.py, because the Trac URL is specified as a config option.

Then, continue testing like PR #3:

Run tests using unittest from the project directory:
python3.8 -m unittest

Checking output

I did not find an easy way to verify the GitHub link behavior. The only way to properly test is to run the conversion, then push and check the wiki.

There are some things you can still check locally:

run the script, and then look at the diff using gitk, with the "Ignore space change" checkbox checked.

check what :trac: markers are left

danuker · 2021-01-27T17:06:32Z

This is ready for review.

I have some questions:

Is there any way to further deduplicate all the _trac[...] methods? They seem to rhyme to me, but could not quite extract another method from sections that repeat.
Are the tiny methods overkill? (_underline, _matches, _sub)? To me they look okay.

danuker · 2021-01-27T17:07:04Z

needs-review

adiroiban

Well done.

We also have the 3rd level of heading ... but I think that this is only in a few pages

8 matches across 8 files

But we need to create a list in README or a script (python or shell) to identify the pages and things that will need manual conversion.

I would like you to continue working on this and have a PR in which the [[PageOutline]] macro is fixed and in which every page will have a local table of content inserted.

And then one final PR in which [[TitleIndex(Development/)]]macro is fixed in the footer... and we should be ready with the conversion.

I only left minor comments. Feel free to addess as you wish and merge this.

For this one time conversion code I have not looks into details on how the code is written .
I was focused on the tests and they are great :)

The GitHub action integration is more about having you familiarized with GitHub action continuous testing infrastructure ... as we use it a lot.

We still have Buildbot, but the plan is to migrate to GitHub actions.

Thanks!

adiroiban · 2021-01-27T17:40:34Z

test/test_wiki_trac_rst_convert.py

+        Process general links from TracWiki format to plain RST links
+        """
+        self.assertConvertedContent(
+            '`Buildbot <https://chevah.com/buildbot/>`_\n',


just a minor comment.
I think that assertConvertedContent should be updated to automatically add the expected newline.

Otherwise these tests are confusing.

You have input as [wiki:"Administrative"] without newlines and then the result has a newline.

I know why there is that newline, but the newline is not part of the conversion for that link.

adiroiban · 2021-01-27T17:45:10Z

test/test_wiki_trac_rst_convert.py

+    def test_trac_ticket(self):
+        """
+        Trac ticket references are converted to a hyperlink.
+        This use case requires `config.py` with the following setting:


this test is ok here in the context of this project.

but otherwise this tests is fragile as it is not "self-sufficient"

It relies that before running the test you rename the sample config.

But this is ok here, and it's not worth fixing the way config works.

Also, just as an exercise. Can you please enable GitHub Actions automatic test execution for this repo?

Maybe in a separate PR.

adiroiban · 2021-01-27T17:46:18Z

test/test_wiki_trac_rst_convert.py

+            'Heading\n=======\n',
+            '= Heading ='


Just a bit easier to read ... you write the code a few times but read it many times... and often while you are tired and distracted by some bug or something that you don't understand :)

Suggested change

'Heading\n=======\n',

'= Heading ='

'Heading\n'

'=======\n',

'= Heading ='

adiroiban · 2021-01-27T17:47:14Z

test/test_wiki_trac_rst_convert.py

+            'Policy and Process\n'
+            '==================\n\n'
+            'Some text\n',
+
+            '= Policy and Process =\n\n'
+            'Some text'


Suggested change

'Policy and Process\n'

'==================\n\n'

'Some text\n',

'= Policy and Process =\n\n'

'Some text'

'Policy and Process\n'

'==================\n'

'\n'

'Some text\n',

'= Policy and Process =\n'

'\n'

'Some text'

adiroiban · 2021-01-27T17:48:50Z

test/test_wiki_trac_rst_convert.py

+        Separates lists from paragraphs by one empty line.
+
+        In RST, when lists follow a paragraph without an empty line
+        inbetween, they fail to parse as lists.


I think that RST is expecting to have a colon (:) at the end of the paragraph preceeding a list...
Maybe auto-add one ?

but this is also ok.

I will leave it as is (add an empty line between), because GitHub does some very strange things with the syntax.

It either:

(not pictured) if no newline nor one-space indent exists before list items, it does not parse it as a list, just shows the plaintext asterisks and list items continuing on the same line, or

if there is a one-space indent, it makes the preceding sentence bold and italic:

adiroiban · 2021-01-27T17:51:21Z

test/test_wiki_trac_rst_convert.py

+
+    def test_bold_is_not_list(self):
+        """
+        Does not add an empty line before a line starting with italic text.


Maybe a more generic description for the test.

I know that this test was caused by a bug where an empty line might have been introduced.

When possible the test should be written descripting a feature and not describing a bug :)

Suggested change

Does not add an empty line before a line starting with italic text.

Italic text markup is preserved as in the TracWiki format.

adiroiban · 2021-01-27T18:02:22Z

test/test_wiki_trac_rst_convert.py

+            'Some text\n',
+
+            '== Subheading ==\n\n'
+            'Some text'


Just a minor test that is missing.

Conversion from source

== Subheading == Test right below the heading

It would be nice to convert it as something like this with one extra newline added...

Subheading ---------- Test right below the heading

but this conversion is also ok

Subheading ---------- Test right below the heading

no need to spend more time here

The test right below this one (with "Some text") ensures two \ns already. But I will format it like you suggested for the 1st level heading test, since it is clear that the \ns on the same line are confusing :)

I wondered what black does. It's a problem, but they are aware of it. Look what it made of the two tests:

self.assertConvertedContent("Subheading\n" "----------", "== Subheading ==") self.assertConvertedContent( "Subheading\n" "----------\n" "\n" "Some text", "== Subheading ==\n" "\n" "Some text", )

adiroiban · 2021-01-27T18:05:28Z

wiki_trac_rst_convert.py

+def _tracwiki_heading_to_rst_heading(text: str):
+    """
+    Convert TracWiki headings (titles surrounded by "=") to RST (titles
+    underlined with "=").
+    """


Maybe use some simple example in the docstring to explain how the conversion is done.

No need for all the details... we can have corner cases explained in the test code.

Suggested change

def _tracwiki_heading_to_rst_heading(text: str):

"""

Convert TracWiki headings (titles surrounded by "=") to RST (titles

underlined with "=").

"""

def _tracwiki_1st_heading_to_rst_heading(text: str):

"""

Convert TracWiki 1st level headings to RST heading.

TracWiki:

= Some Top Heading =

Content here

RST conversion:

Some Top Heading

----------------

Content here

"""

adiroiban · 2021-01-27T18:10:56Z

test/test_wiki_trac_rst_convert.py

@@ -3,10 +3,10 @@
 from wiki_trac_rst_convert import convert_content


-class TracRstToVanillaRst(unittest.TestCase):
+class TracToGithubRst(unittest.TestCase):


naming is complicated ... but I think that the correct speling is GitHub ...also, when we have an abreviation like RST or URL, we keep upper case in the Python name.

so we prefer getURL and not getUrl

Suggested change

class TracToGithubRst(unittest.TestCase):

class TracToGitHubRST(unittest.TestCase):

danuker linked an issue Jan 26, 2021 that may be closed by this pull request

Convert TracWiki syntax to GitHub-friendly RST #4

Closed

WIP ticket #4: TracWiki-format wiki and ticket links

c869b17

danuker force-pushed the 4_tracwiki_to_github branch from 42a4292 to c869b17 Compare January 26, 2021 15:16

chevah-robot added the needs-review label Jan 27, 2021

chevah-robot assigned adiroiban Jan 27, 2021

danuker marked this pull request as ready for review January 27, 2021 17:07

danuker changed the title ~~WIP: TracWiki links and formatting (#4)~~ TracWiki links and formatting (#4) Jan 27, 2021

adiroiban approved these changes Jan 27, 2021

View reviewed changes

chevah-robot added needs-merge and removed needs-review labels Jan 27, 2021

chevah-robot assigned danuker and unassigned adiroiban Jan 27, 2021

danuker and others added 5 commits January 28, 2021 12:47

Process TracWiki format: links, headings, and lists

5cca393

Update readme with todo or manual edits.

e42cc6c

Add example for nested sidebar.

a067e37

Fix list formating.

732af01

Add info about GitHub meta pages.

cfb4135

danuker force-pushed the 4_tracwiki_to_github branch from e34276e to cfb4135 Compare January 28, 2021 11:01

danuker changed the title ~~TracWiki links and formatting (#4)~~ Process TracWiki links and formatting. (#4) Jan 28, 2021

danuker merged commit f1c19a7 into main Jan 28, 2021

adiroiban deleted the 4_tracwiki_to_github branch January 28, 2021 14:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Process TracWiki links and formatting. (#4) #6

Process TracWiki links and formatting. (#4) #6

danuker commented Jan 26, 2021 •

edited

Loading

danuker commented Jan 27, 2021

danuker commented Jan 27, 2021

adiroiban left a comment

adiroiban Jan 27, 2021

adiroiban Jan 27, 2021

adiroiban Jan 27, 2021

adiroiban Jan 27, 2021

adiroiban Jan 27, 2021

danuker Jan 28, 2021 •

edited

Loading

adiroiban Jan 27, 2021

adiroiban Jan 27, 2021

danuker Jan 28, 2021 •

edited

Loading

adiroiban Jan 27, 2021

adiroiban Jan 27, 2021

	Does not add an empty line before a line starting with italic text.
	Italic text markup is preserved as in the TracWiki format.

-def _tracwiki_heading_to_rst_heading(text: str):
-    """
-    Convert TracWiki headings (titles surrounded by "=") to RST (titles
-    underlined with "=").
-    """
+def _tracwiki_1st_heading_to_rst_heading(text: str):
+    """
+    Convert TracWiki  1st level headings to RST heading.
+    TracWiki:
+    = Some Top Heading =
+    Content here
+    RST conversion:
+    Some Top Heading
+    ----------------
+    Content here
+    """

	class TracToGithubRst(unittest.TestCase):
	class TracToGitHubRST(unittest.TestCase):

Process TracWiki links and formatting. (#4) #6

Process TracWiki links and formatting. (#4) #6

Conversation

danuker commented Jan 26, 2021 • edited Loading

Scope

Changes

What is not done

How to try and test the changes

Checking output

danuker commented Jan 27, 2021

danuker commented Jan 27, 2021

adiroiban left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danuker Jan 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danuker Jan 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danuker commented Jan 26, 2021 •

edited

Loading

danuker Jan 28, 2021 •

edited

Loading

danuker Jan 28, 2021 •

edited

Loading