1389956 - ./mach clang-format - Improvement of the code

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Sylvestre Ledru [:Sylvestre]]

Follow up from bug 1387036

instead of 
 if os.path.exists(".hg"):
            diff_process = Popen(["hg", "diff", "-U0", "-r", ".^",
                                  "--include", "glob:**.c", "--include", "glob:**.cpp",
                                  "--include", "glob:**.h",
                                  "--exclude", "listfile:.clang-format-ignore"], stdout=PIPE)
        else:
	    diff_process = Popen(["git", "diff", "--no-color", "-U0", "HEAD","--","*.c","*.cpp","*.h"], stdout=PIPE)

we could have something like
1) create a simple datastructure:
filter=("cpp","c","h")

2) generate the command line in a function depending on the vcs like
cmd = generateDiffCommandVCS()

3) having a singel call to Popen(cmd)

Comment 1

[Sylvestre Ledru [:Sylvestre]]

also,  we also have extensions = ('.cpp', '.c', '.h') which should be used/moved
https://dxr.mozilla.org/mozilla-central/source/tools/mach_commands.py#286

Comment 2

[Xavier ALT [:dex]]

Hi Sylvestre, i would like to work on this bug, could you assign it to me ? Thanks!

Comment 3

[Sylvestre Ledru [:Sylvestre]]

sure, done!

Comment 4

[Xavier ALT [:dex]]

Attachment: Bug 1389956 - extract clang format diff command generation into its own method

Review commit: https://reviewboard.mozilla.org/r/171484/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171484/

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Xavier, you are implementing several changes at once (this bug and bug 1392012)
Could you please separate the two?

Comment 6

[Xavier ALT [:dex]]

Comment on attachment 8900110 [details]
Bug 1389956 - extract clang format diff command generation into its own method

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171484/diff/1-2/

Comment 7

[Xavier ALT [:dex]]

Hi Sylvestre, I've rebased my change and splitted this in two.

The patch/review for bug 1392012 still depend on the changes of the current bug, let me known if you prefer to fix bug 1392012 before the current one, so I can update the patch accordingly.

Thanks

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8900110 [details]
Bug 1389956 - extract clang format diff command generation into its own method

https://reviewboard.mozilla.org/r/171484/#review178454

Great, thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6803e175c0f3
extract clang format diff command generation into its own method r=sylvestre

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6803e175c0f3