Created attachment 128571 [details] [review]
Bug 571483 - Sort toplevel .gir entries

This ensures we're stable on a macro level.

Comment on attachment 128571 [details] [review]
Bug 571483 - Sort toplevel .gir entries

>From aac482fe6bb8f152e82688deca2976e4a7da33f5 Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Thu, 12 Feb 2009 10:12:01 -0500
>Subject: [PATCH] Bug 571483 - Sort toplevel .gir entries
>
>This ensures we're stable on a macro level.
>---
> giscanner/ast.py                           |    3 +
> giscanner/girwriter.py                     |   12 +-
> tests/scanner/annotation-1.0-expected.gir  |   64 ++--
> tests/scanner/annotation-1.0-expected.tgir |   54 ++--
> tests/scanner/foo-1.0-expected.gir         |  694 ++++++++++++++--------------
> tests/scanner/foo-1.0-expected.tgir        |  498 ++++++++++----------
> tests/scanner/utility-1.0-expected.gir     |  104 ++--
> tests/scanner/utility-1.0-expected.tgir    |   66 ++--
> 8 files changed, 754 insertions(+), 741 deletions(-)
>
>diff --git a/giscanner/ast.py b/giscanner/ast.py
>index 14cd1a6..ff65312 100644
>--- a/giscanner/ast.py
>+++ b/giscanner/ast.py
>@@ -150,6 +150,9 @@ class Node(object):
>         self.deprecated_version = None
>         self.version = None
> 
>+    def __cmp__(self, other):
>+        return cmp(self.name, other.name)
>+
>     def __repr__(self):
>         return '%s(%r)' % (self.__class__.__name__, self.name)
> 
>diff --git a/giscanner/girwriter.py b/giscanner/girwriter.py
>index 35c9b35..3577cf1 100644
>--- a/giscanner/girwriter.py
>+++ b/giscanner/girwriter.py
>@@ -80,7 +80,17 @@ and/or use gtk-doc annotations. ''')
>                  ('version', namespace.version),
>                  ('shared-library', ','.join(libraries))]
>         with self.tagcontext('namespace', attrs):
>-            for node in namespace.nodes:
>+            def nscmp(a, b):
>+                if isinstance(a, Alias):
>+                    if isinstance(b, Alias):
>+                        return cmp(a.name, b.name)
>+                    else:
>+                        return -1
>+                elif isinstance(b, Alias):
>+                    return 1
>+                else:
>+                    return cmp(a, b)

Can't you just override __cmp__ or __eq__ in Alias to avoid this?

I guess in my thought process was something of the form that putting the aliases first was a sort of aesthetic concern for the .gir format, building on top of a generic __cmp__ method for Node.

I don't have a strong opinion though; you think the logic to sort aliases first should go in Alias?  If so I'm fine moving it.

Hmm, maybe I'm confused. There's no real need to be the aliases before other nodes right? I thought you just needed different sorting logic for the aliases themselves.

They are special in that the typelib compiler needs to parse all aliases first (c.f. girparser.c:firstpass_parser).  Though we still do parse the whole file in that first pass, so it's not an optimization to have them first, just feels cleaner to me.

Fine, that last comment seems like a good comment to include in girwriter explaining the alias sorting thing.

Thanks for review, added a comment.

commit 5444b63da235cf8e43da7352556de04943354a56
Author: Colin Walters <walters@verbum.org>
Date:   Thu Feb 12 10:12:01 2009 -0500

    Bug 571483 - Sort toplevel .gir entries
    
    This ensures we're stable on a macro level.

[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]