ros-infrastructure / catkin_pkg

Standalone Python library for the catkin build system.
https://github.com/ros/catkin
Other
47 stars 91 forks source link

more detailed error message those who wrongly using run_depend on version 2 #246

Closed k-okada closed 5 years ago

k-okada commented 5 years ago

Added more detailed warnings when people uses run_depend on package.xml v2 or exec_depend on package.xml v1.

    xml, filename=filename, warnings=warnings)
  File "/home/k-okada/catkin_ws/ws_catkin_pkg/venv/lib/python2.7/site-packages/catkin_pkg-0.4.8-py2.7.egg/catkin_pkg/package.py", line 705, in parse_package_string
    raise InvalidPackage('Error(s):%s' % (''.join(['\n- %s' % e for e in errors])), filename)
catkin_pkg.package.InvalidPackage: Error(s) in package '/home/k-okada/catkin_ws/ws_catkin_pkg/src/test_pkg_2/package.xml':
Error(s):
- WARNING:
- WARNING: ACTION REQUIRED
- WARNING:
- WARNING: The manifest of package "test_pkg_2" (with format version 2) does not support <run_depend>. 
- WARNING: Please update your "test_pkg_2/package.xml" with <exec_depend>
- WARNING:
- WARNING:

c.f. #238

k-okada commented 5 years ago

@dirk-thomas thanks for comments. I have fixed and committed newer version.

BTW, I still want to emphasize error message so that (specially) beginners do not confused with 'Traceback' and 'Error' message. How do you think raising 'InvalidPackage' exception with color formatted string.

index 9d49e61..d7f8269 100644
--- a/src/catkin_pkg/package.py
+++ b/src/catkin_pkg/package.py
@@ -41,6 +41,7 @@ import sys
 import xml.dom.minidom as dom

 from catkin_pkg.condition import evaluate_condition
+from catkin_pkg.terminal_color import fmt

 PACKAGE_MANIFEST_FILENAME = 'package.xml'

@@ -714,7 +715,7 @@ def parse_package_string(data, filename=None, warnings=None):
                 errors.append('The "%s" tag must not contain the following children: %s' % (node.tagName, ', '.join([n.tagName for n in subnodes])))

     if errors:
-        raise InvalidPackage('Error(s):%s' % (''.join(['\n- %s' % e for e in errors])), filename)
+        raise InvalidPackage(fmt('@{rf}@{boldon}Error(s):%s' % (''.join(['\n- %s' % e for e in errors]))), filename)
k-okada commented 5 years ago

Sorry , i have closed this issue by force push origin/master to k-okada/master, moved PR to #248