Merge pull request #89 from google/sections

Improve section/order handling

Author: dbarnett

tests/__init__.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()))

tests/module_tests.py

@@ -16,14 +16,15 @@ 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.
     is_default: Whether other blocks with the same type and tag should override
         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 @@ def __init__(self, is_secondary=False, is_default=False):
     # 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)

vimdoc/block.py

@@ -116,6 +116,18 @@ def __init__(self, section):
         '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__(

vimdoc/error.py

@@ -60,11 +60,17 @@ def Merge(self, block, namespace=None):
       # 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 @@ def Close(self):
     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 @@ def Close(self):
     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 @@ def Chunks(self):
       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 @@ def LookupTag(self, typ, name):
         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)