On 07/15/2012 08:53 AM, Scott Talbert wrote:
Hi Stephen,
I have been contributing to Concordance lately, and I have written a
patch for Congruity to port it to the new libconcord API. If you have
some time, I would greatly appreciate it if you could review the patch,
which I have attached. Details on my changes are listed below.
Sorry for the slow response; I've been away on vacation.
I've quoted the attachment below.
A lot of my feedback is more about the libconcord API changes than the
congruity patch itself.
+import threading
It'd be better to simply consistently use thread rather than mixing
thread and threading.
-version = 15+
+version = 16+
Functional patches shouldn't touch the version; it should only be bumped
during or right after release.
-# Fix typo in libconcord 0.20 Python bindings
+# Test to ensure we have the new API available.
try:
-libconcord.delete_blob
+libconcord.update_configuration
except:
-libconcord.delete_blob = libconcord.delete_block
+str = traceback.format_exc()
+app = wx.PySimpleApp()
+dlg = wx.MessageDialog(
+None,
+Could not load the correct version of libconcord; please ensure
+that the latest version is installed.\n\n + str,
+congruity: Dependency Error,
+wx.OK | wx.ICON_ERROR
+)
+dlg.ShowModal()
+os._exit(1)
Hmm. I've made sure that congruity to date can run with almost any
version of libconcord. That change breaks this feature. Isn't there a
way to introduce support for the new libconcord APIs without making it
non-backwards-compatible?
-def program_callback_imp(count, current, total, context):
+def program_callback_imp(stage_id, count, current, total, type, context):
if not context:
return
+if stage_id == libconcord.LC_CB_STAGE_NUM_STAGES:
+return
+
try:
-(f, fcontext) = context
-percent = (current * 100) / total
-f(False, percent, fcontext)
+if isinstance(context, ProgramRemotePanelBase):
That's not very object-oriented; it'd be better to call a
function/method in the context object, and have that function do
whatever is appropriate, rather than having the code query the type of
the context and then act differently.
+if stage_id not in context.dg_widgets:
+context.dg_widget_added = threading.Event()
+wx.CallAfter(context._AddDg, stage_id)
+context.dg_widget_added.wait()
+context._DgStart(context.dg_widgets[stage_id])
Uggh. I'll comment on that later.
@@ -379,54 +365,7 @@ class WelcomePanel(MessagePanelBase):
self.next = self.resources.page_connect
-xml = POINTER(c_ubyte)()
-xml_size = c_uint()
-libconcord.read_file(
-self.resources.ezhex_filename,
-byref(xml),
-byref(xml_size)
-)
-self.resources.SetXmlData(xml, xml_size)
-
-type = c_int()
-libconcord.identify_file(xml, xml_size, byref(type))
Uggh. Why can't libconcord parse the file without being attached to the
remote; the file format should be completely standalone and parse-able
offline.
@@ -688,7 +694,6 @@ class ProgramRemotePanelBase(WizardPanelBase, Deco
self.dc = DecoratedContainer(self, self.resources)
DecoratedContainerThreadMixin.__init__(self, self.dc)
-self._AddWidgets()
self.sizer.Add(self.dc, 0, wx.EXPAND | wx.ALL, 5)
This and the related changes will cause a horrible regression in the
utility of the GUI; the poor user will just see more and more tasks
randomly appearing, and have no idea when the end is in sight. congruity
may as well embed a vte (terminal) widget and just run the concordance
application in it given this change. I've complained about this
libconcord API change repeatedly before...
That all said, I don't really have much time or motivation for congruity
work these days, so I'm not going to really push back hard on these changes.
If you want, perhaps I can hand off congruity maintenance to you since
you've obviously got some interest in it; I assume that Sourceforge will
let me set up other committers/admins. Let me know if you want, and what
your Sourceforge login ID is.
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel
