Opened 6 years ago

Closed 6 years ago

#3017 enhancement closed fixed (fixed)

man2lore doesn't correctly translate .TP tags

Reported by: therve Owned by:
Priority: highest Milestone:
Component: lore Keywords:
Cc: Branch: branches/fix-man2lore-3017
(diff, github, buildbot, log)
Author: therve Launchpad Bug:

Description

Basically, it means that the output of the list of options in xhtml is broken.

Change History (12)

comment:1 Changed 6 years ago by therve

  • author set to therve
  • Branch set to branches/fix-man2lore-3017

(In [22432]) Branching to 'fix-man2lore-3017'

comment:2 Changed 6 years ago by therve

(In [22433]) Fix and tests.

Refs #3017

comment:3 Changed 6 years ago by therve

  • Keywords review added
  • Owner therve deleted

This is ready to review. man2lore would need one hundred time more tests, but I'm not sure I have the energy to do that right now. As it's a release blocker...

comment:4 Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner set to therve

eyes bleeding, etc. Thanks for tackling this. I won't ask for all the extra tests this code really needs, since the change to fix this issue is pretty small and the approach man2lore takes is completely wrong anyway...

Some stuff I think should be done, though:

  • test-case-name for man2lore.py
  • docstring for macro_TP and text methods of ManConverter, since they had changes
  • documentation for the tp ivar of ManConverter
  • Factor the common parts of the tests into a helper - writing to a file, calling convert, etc.

comment:5 Changed 6 years ago by therve

(In [22439]) Process review comments.

Refs #3017.

comment:6 Changed 6 years ago by therve

  • Keywords review added
  • Owner therve deleted
  • Priority changed from normal to highest

Thanks for the review. I think I fix everything, except the test-case-name which was already there :).

comment:7 Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner set to therve

Cool, thanks. ManConverter.tp is still pretty opaque to me though. Do you think you could document what each of the 4 values it can take on mean (I ask because I think you must have figured out at least some of them in order to make this change :)? This could either take the form of an expanded @ivar or maybe the constants could be replaced with symbolic names which have some meaning.

comment:8 Changed 6 years ago by therve

(In [22459]) Document tp variable.

Refs #3017

comment:9 Changed 6 years ago by therve

  • Keywords review added
  • Owner therve deleted

I choosed the verbose way, I hope this is clear enough. Back to review.

comment:10 Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner set to therve

Looks good, thanks. Please merge.

comment:11 Changed 6 years ago by therve

  • Resolution set to fixed
  • Status changed from new to closed

(In [22460]) Merge fix-man2lore-3017

Author: therve
Reviewer: exarkun
Fixes #3017

Fix the conversion of the list of options in man pages to Lore format.

comment:12 Changed 3 years ago by <automation>

  • Owner therve deleted
Note: See TracTickets for help on using tickets.