[1/4] tools: moveconfig: trim garbage lines after header cleanups

Message ID 1469369859-14277-2-git-send-email-yamada.masahiro@socionext.com
State New
Headers show

Commit Message

Masahiro Yamada July 24, 2016, 2:17 p.m.
The tools/moveconfig.py has a feature to cleanup #define/#undef's
of moved config options, but I want this tool to do a better job.

For example, when we are moving CONFIG_FOO and its define is
surrounded by #ifdef ... #endif, like follows:

  #ifdef CONFIG_BAR
  #  define CONFIG_FOO
  #endif

The header cleanup will leave empty #ifdef ... #endif:

  #ifdef CONFIG_BAR
  #endif

Likewise, if a define line between two blank lines

  <blank line>
  #define CONFIG_FOO
  <blank lines.

... is deleted, the result of the clean-up will be successive empty
lines, which is a coding-style violation.

It is tedious to remove left-over garbage lines manually, so I want
the tool to take care of this.  The tool's job is still not perfect,
so we should check the output of the tool, but I hope our life will
be much easier with this patch.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 tools/moveconfig.py | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 4 deletions(-)

-- 
1.9.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Masahiro Yamada July 25, 2016, 10:20 a.m. | #1
Hi Tom,


2016-07-25 0:19 GMT+09:00 Tom Rini <trini@konsulko.com>:
> On Sun, Jul 24, 2016 at 11:17:36PM +0900, Masahiro Yamada wrote:

>

>> The tools/moveconfig.py has a feature to cleanup #define/#undef's

>> of moved config options, but I want this tool to do a better job.

>>

>> For example, when we are moving CONFIG_FOO and its define is

>> surrounded by #ifdef ... #endif, like follows:

>>

>>   #ifdef CONFIG_BAR

>>   #  define CONFIG_FOO

>>   #endif

>>

>> The header cleanup will leave empty #ifdef ... #endif:

>>

>>   #ifdef CONFIG_BAR

>>   #endif

>>

>> Likewise, if a define line between two blank lines

>>

>>   <blank line>

>>   #define CONFIG_FOO

>>   <blank lines.

>>

>> ... is deleted, the result of the clean-up will be successive empty

>> lines, which is a coding-style violation.

>

> Thanks!  This is mildly annoying to find/fixup with regex, which is how

> I usually do it.  A few comments / questions:

>

> [snip]

>> +    # remove empty #ifdef ... #endif, successive blank lines

>> +    pattern_ifdef = re.compile(r'#\s*if(def|ndef)?\s')

>

> It starts to get complex to catch if defined/elif defined cases, so

> we should just leave that to manual review, yes?


It was missing from my thought.

It is difficult to invert the logic, like changing ifdef to ifndef,
but just removing can be done easily.

In v2,

"tools/moveconfig -H -c FOO" can clean more!


ex1)

   #if CONFIG_BAR
     /* ... */
  -#elif CONFIG_BAZ
  -# define CONFIG_FOO
   #endif


ex2)
   #if CONFIG_BAR
     /* ... */
  -#elif CONFIG_BAZ
  -# define CONFIG_FOO
   #elif CONFIG_BAZ2
     /* ... */
   #endif





>> +    pattern_endif = re.compile(r'#\s*endif\s')

>> +    pattern_blank = re.compile(r'^\s*$')

>

> The last pattern will catch multiple blank lines, right?


Yes.


-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index d362923..7d018e4 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -160,6 +160,7 @@  To see the complete list of supported options, run
 
 """
 
+import copy
 import filecmp
 import fnmatch
 import multiprocessing
@@ -319,6 +320,41 @@  def update_cross_compile(color_enabled):
 
         CROSS_COMPILE[arch] = cross_compile
 
+def extend_matched_lines(lines, matched, pre_pattern, post_pattern, extend_pre,
+                         extend_post):
+    """Extend matched lines if desired patterns are found before/after already
+    matched lines.
+
+    Arguments:
+      lines: A list of lines handled.
+      matched: A list of line numbers that have been already matched.
+               (will be updated by this function)
+      pre_pattern: Regular expression that should be matched as preamble.
+      post_pattern: Regular expression that should be matched as postamble.
+      extend_pre: Add the line number of matched preamble to the matched list.
+      extend_post: Add the line number of matched postamble to the matched list.
+    """
+    extended_matched = []
+
+    j = matched[0]
+
+    for i in matched:
+        if i == 0 or i < j:
+            continue
+        j = i
+        while j in matched:
+            j += 1
+        if j >= len(lines):
+            break
+        if pre_pattern.search(lines[i - 1]) and post_pattern.search(lines[j]):
+            if extend_pre:
+                extended_matched.append(i - 1)
+            if extend_post:
+                extended_matched.append(j)
+
+    matched += extended_matched
+    matched.sort()
+
 def cleanup_one_header(header_path, patterns, dry_run):
     """Clean regex-matched lines away from a file.
 
@@ -334,13 +370,35 @@  def cleanup_one_header(header_path, patterns, dry_run):
     matched = []
     for i, line in enumerate(lines):
         for pattern in patterns:
-            m = pattern.search(line)
-            if m:
-                print '%s: %s: %s' % (header_path, i + 1, line),
+            if pattern.search(line):
                 matched.append(i)
                 break
 
-    if dry_run or not matched:
+    if not matched:
+        return
+
+    # remove empty #ifdef ... #endif, successive blank lines
+    pattern_ifdef = re.compile(r'#\s*if(def|ndef)?\s')
+    pattern_endif = re.compile(r'#\s*endif\s')
+    pattern_blank = re.compile(r'^\s*$')
+
+    while True:
+        old_matched = copy.copy(matched)
+        extend_matched_lines(lines, matched, pattern_ifdef, pattern_endif,
+                             True, True)
+        extend_matched_lines(lines, matched, pattern_ifdef, pattern_blank,
+                             False, True)
+        extend_matched_lines(lines, matched, pattern_blank, pattern_endif,
+                             True, False)
+        extend_matched_lines(lines, matched, pattern_blank, pattern_blank,
+                             True, False)
+        if matched == old_matched:
+            break
+
+    for i in matched:
+        print '%s: %s: %s' % (header_path, i + 1, lines[i]),
+
+    if dry_run:
         return
 
     with open(header_path, 'w') as f: