Repository: google/vimdoc

Issue:
More intuitive section/order handling
It's annoying how you don't have to specify section order with the `@order` directive until you add the first custom section, at which point you have to explicitly list every section ID in the order. You have to cross-reference the list of default sections if you want to preserve the default sections and order.

Also, if you have redundant definitions for the same section, vimdoc should complain instead of just picking one at random.

Discussion:

Patch:
diff --git a/vimdoc/block.py b/vimdoc/block.py
index e56c357..03bed17 100644
--- a/vimdoc/block.py
+++ b/vimdoc/block.py
@@ -16,6 +16,7 @@ class Block(object):
   contain metadata statements specifying things like the plugin author, etc.
 
   Args:
+    type: Block type, e.g. vim.SECTION or vim.FUNCTION.
     is_secondary: Whether there are other blocks above this one that describe
         the same item. Only primary blocks should have tags, not secondary
         blocks.
@@ -23,7 +24,7 @@ class Block(object):
         this one and prevent this block from showing up in the docs.
   """
 
-  def __init__(self, is_secondary=False, is_default=False):
+  def __init__(self, type=None, is_secondary=False, is_default=False):
     # May include:
     # deprecated (boolean)
     # dict (name)
@@ -34,6 +35,8 @@ class Block(object):
     # namespace (of function)
     # attribute (of function in dict)
     self.locals = {}
+    if type is not None:
+      self.SetType(type)
     # Merged into module. May include:
     # author (string)
     # library (boolean)
diff --git a/vimdoc/error.py b/vimdoc/error.py
index ead432b..127ad53 100644
--- a/vimdoc/error.py
+++ b/vimdoc/error.py
@@ -116,6 +116,18 @@ class NoSuchSection(BadStructure):
         'Section {} never defined.'.format(section))
 
 
+class DuplicateSection(BadStructure):
+  def __init__(self, section):
+    super(DuplicateSection, self).__init__(
+        'Duplicate section {} defined.'.format(section))
+
+
+class DuplicateBackmatter(BadStructure):
+  def __init__(self, section):
+    super(DuplicateBackmatter, self).__init__(
+        'Duplicate backmatter defined for section {}.'.format(section))
+
+
 class NeglectedSections(BadStructure):
   def __init__(self, sections, order):
     super(NeglectedSections, self).__init__(
diff --git a/vimdoc/module.py b/vimdoc/module.py
index 6e76c4e..05b09d1 100644
--- a/vimdoc/module.py
+++ b/vimdoc/module.py
@@ -60,11 +60,17 @@ class Module(object):
       # Overwrite existing section if it's a default.
       if block_id not in self.sections or self.sections[block_id].IsDefault():
         self.sections[block_id] = block
+      elif not block.IsDefault():
+        # Tried to overwrite explicit section with explicit section.
+        raise error.DuplicateSection(block_id)
     elif typ == vimdoc.BACKMATTER:
       # Overwrite existing section backmatter if it's a default.
       if (block_id not in self.backmatters
           or self.backmatters[block_id].IsDefault()):
         self.backmatters[block_id] = block
+      elif not block.IsDefault():
+        # Tried to overwrite explicit backmatter with explicit backmatter.
+        raise error.DuplicateBackmatter(block_id)
     else:
       collection_type = self.plugin.GetCollectionType(block)
       if collection_type is not None:
@@ -107,31 +113,26 @@ class Module(object):
     All default sections that have not been overridden will be created.
     """
     if self.GetCollection(vimdoc.FUNCTION) and 'functions' not in self.sections:
-      functions = Block()
-      functions.SetType(vimdoc.SECTION)
+      functions = Block(vimdoc.SECTION)
       functions.Local(id='functions', name='Functions')
       self.Merge(functions)
     if (self.GetCollection(vimdoc.EXCEPTION)
         and 'exceptions' not in self.sections):
-      exceptions = Block()
-      exceptions.SetType(vimdoc.SECTION)
+      exceptions = Block(vimdoc.SECTION)
       exceptions.Local(id='exceptions', name='Exceptions')
       self.Merge(exceptions)
     if self.GetCollection(vimdoc.COMMAND) and 'commands' not in self.sections:
-      commands = Block()
-      commands.SetType(vimdoc.SECTION)
+      commands = Block(vimdoc.SECTION)
       commands.Local(id='commands', name='Commands')
       self.Merge(commands)
     if self.GetCollection(vimdoc.DICTIONARY) and 'dicts' not in self.sections:
-      dicts = Block()
-      dicts.SetType(vimdoc.SECTION)
+      dicts = Block(vimdoc.SECTION)
       dicts.Local(id='dicts', name='Dictionaries')
       self.Merge(dicts)
     if self.GetCollection(vimdoc.FLAG):
       # If any maktaba flags were documented, add a default configuration
       # section to explain how to use them.
-      config = Block(is_default=True)
-      config.SetType(vimdoc.SECTION)
+      config = Block(vimdoc.SECTION, is_default=True)
       config.Local(id='config', name='Configuration')
       config.AddLine(
           'This plugin uses maktaba flags for configuration. Install Glaive'
@@ -141,29 +142,18 @@ class Module(object):
     if ((self.GetCollection(vimdoc.FLAG) or
          self.GetCollection(vimdoc.SETTING)) and
         'config' not in self.sections):
-      config = Block()
-      config.SetType(vimdoc.SECTION)
+      config = Block(vimdoc.SECTION)
       config.Local(id='config', name='Configuration')
       self.Merge(config)
-    if not self.order:
-      self.order = []
-      for builtin in [
-          'intro',
-          'config',
-          'commands',
-          'autocmds',
-          'settings',
-          'dicts',
-          'functions',
-          'exceptions',
-          'mappings',
-          'about']:
-        if builtin in self.sections or builtin in self.backmatters:
-          self.order.append(builtin)
+
     for backmatter in self.backmatters:
       if backmatter not in self.sections:
         raise error.NoSuchSection(backmatter)
-    known = set(self.sections) | set(self.backmatters)
+    # Use explicit order as partial ordering and merge with default section
+    # ordering. All custom sections must be ordered explicitly.
+    self.order = self._GetSectionOrder(self.order, self.sections)
+
+    known = set(self.sections)
     neglected = sorted(known.difference(self.order))
     if neglected:
       raise error.NeglectedSections(neglected, self.order)
@@ -200,6 +190,46 @@ class Module(object):
       if ident in self.backmatters:
         yield self.backmatters[ident]
 
+  @staticmethod
+  def _GetSectionOrder(explicit_order, sections):
+    """Gets final section order from explicit_order and actual sections present.
+
+    Built-in sections with no explicit order come before custom sections, with
+    two exceptions:
+      * The "about" section comes last by default.
+      * If a built-in section is explicitly ordered, it "resets" the ordering so
+        so that subsequent built-in sections come directly after it.
+    This yields the order you would intuitively expect in cases like ordering
+    "intro" after other sections.
+    """
+    order = explicit_order or []
+    default_order = [
+      'intro',
+      'config',
+      'commands',
+      'autocmds',
+      'settings',
+      'dicts',
+      'functions',
+      'exceptions',
+      'mappings']
+    # Add any undeclared sections before custom sections, except 'about' which
+    # comes at the end by default.
+    section_insertion_idx = 0
+    order = order[:]
+    for builtin in default_order:
+      if builtin in order:
+        # Section already present. Skip and continue later sections after it.
+        section_insertion_idx = order.index(builtin) + 1
+        continue
+      else:
+        # If section present, insert into order at logical index.
+        if builtin in sections:
+          order.insert(section_insertion_idx, builtin)
+          section_insertion_idx += 1
+    if 'about' in sections and 'about' not in order:
+      order.append('about')
+    return order
 
 class VimPlugin(object):
   """State for entire plugin (potentially multiple modules)."""
@@ -249,8 +279,7 @@ class VimPlugin(object):
         block = candidates[0]
     if block is None:
       # Create a dummy block to get default tag.
-      block = Block()
-      block.SetType(typ)
+      block = Block(typ)
       block.Local(name=fullname)
     return block.TagName()
 
@@ -353,8 +382,7 @@ def Modules(directory):
               flagpath = relative_path
               if flagpath.startswith('after' + os.path.sep):
                 flagpath = os.path.relpath(flagpath, 'after')
-              flagblock = Block(is_default=True)
-              flagblock.SetType(vimdoc.FLAG)
+              flagblock = Block(vimdoc.FLAG, is_default=True)
               name_parts = os.path.splitext(flagpath)[0].split(os.path.sep)
               flagname = name_parts.pop(0)
               flagname += ''.join('[' + p + ']' for p in name_parts)


Test patch:
diff --git a/tests/__init__.py b/tests/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/module_tests.py b/tests/module_tests.py
new file mode 100644
index 0000000..1226997
--- /dev/null
+++ b/tests/module_tests.py
@@ -0,0 +1,92 @@
+import unittest
+
+import vimdoc
+from vimdoc.block import Block
+from vimdoc import error
+from vimdoc import module
+
+class TestVimModule(unittest.TestCase):
+
+  def test_section(self):
+    plugin = module.VimPlugin('myplugin')
+    main_module = module.Module('myplugin', plugin)
+    intro = Block(vimdoc.SECTION)
+    intro.Local(name='Introduction', id='intro')
+    main_module.Merge(intro)
+    main_module.Close()
+    self.assertEqual([intro], list(main_module.Chunks()))
+
+  def test_duplicate_section(self):
+    plugin = module.VimPlugin('myplugin')
+    main_module = module.Module('myplugin', plugin)
+    intro = Block(vimdoc.SECTION)
+    intro.Local(name='Introduction', id='intro')
+    main_module.Merge(intro)
+    intro2 = Block(vimdoc.SECTION)
+    intro2.Local(name='Intro', id='intro')
+    with self.assertRaises(error.DuplicateSection) as cm:
+      main_module.Merge(intro2)
+    self.assertEqual(('Duplicate section intro defined.',), cm.exception.args)
+
+  def test_default_section_ordering(self):
+    """Sections should be ordered according to documented built-in ordering."""
+    plugin = module.VimPlugin('myplugin')
+    main_module = module.Module('myplugin', plugin)
+    intro = Block(vimdoc.SECTION)
+    intro.Local(name='Introduction', id='intro')
+    commands = Block(vimdoc.SECTION)
+    commands.Local(name='Commands', id='commands')
+    about = Block(vimdoc.SECTION)
+    about.Local(name='About', id='about')
+    # Merge in arbitrary order.
+    main_module.Merge(commands)
+    main_module.Merge(about)
+    main_module.Merge(intro)
+    main_module.Close()
+    self.assertEqual([intro, commands, about], list(main_module.Chunks()))
+
+  def test_manual_section_ordering(self):
+    """Sections should be ordered according to explicitly configured order."""
+    plugin = module.VimPlugin('myplugin')
+    main_module = module.Module('myplugin', plugin)
+    intro = Block(vimdoc.SECTION)
+    intro.Local(name='Introduction', id='intro')
+    # Configure explicit order.
+    intro.Global(order=['commands', 'about', 'intro'])
+    commands = Block(vimdoc.SECTION)
+    commands.Local(name='Commands', id='commands')
+    about = Block(vimdoc.SECTION)
+    about.Local(name='About', id='about')
+    # Merge in arbitrary order.
+    main_module.Merge(commands)
+    main_module.Merge(about)
+    main_module.Merge(intro)
+    main_module.Close()
+    self.assertEqual([commands, about, intro], list(main_module.Chunks()))
+
+  def test_partial_ordering(self):
+    """Always respect explicit order and prefer built-in ordering.
+
+    Undeclared built-in sections will be inserted into explicit order according
+    to default built-in ordering. The about section should come after custom
+    sections unless explicitly ordered."""
+    plugin = module.VimPlugin('myplugin')
+    main_module = module.Module('myplugin', plugin)
+    intro = Block(vimdoc.SECTION)
+    intro.Local(name='Introduction', id='intro')
+    # Configure explicit order.
+    intro.Global(order=['custom1', 'intro', 'custom2'])
+    commands = Block(vimdoc.SECTION)
+    commands.Local(name='Commands', id='commands')
+    about = Block(vimdoc.SECTION)
+    about.Local(name='About', id='about')
+    custom1 = Block(vimdoc.SECTION)
+    custom1.Local(name='Custom1', id='custom1')
+    custom2 = Block(vimdoc.SECTION)
+    custom2.Local(name='Custom2', id='custom2')
+    # Merge in arbitrary order.
+    for section in [commands, custom2, about, intro, custom1]:
+      main_module.Merge(section)
+    main_module.Close()
+    self.assertEqual([custom1, intro, commands, custom2, about],
+        list(main_module.Chunks()))