./mach clang-format - Improvement of the code

RESOLVED FIXED in Firefox 57

Status

enhancement
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: sylvestre, Assigned: dex, Mentored)

Tracking

({good-first-bug})

Trunk
mozilla57
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [good first bug][lang=python])

Attachments

(1 attachment)

Reporter

Description

2 years ago
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)
Reporter

Updated

2 years ago
Summary: ./mach clang-format - Improvment of the code → ./mach clang-format - Improvement of the code
Reporter

Comment 1

2 years ago
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
Assignee

Comment 2

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

Comment 3

2 years ago
sure, done!
Assignee: nobody → dex
Comment hidden (mozreview-request)
Reporter

Comment 5

2 years ago
Xavier, you are implementing several changes at once (this bug and bug 1392012)
Could you please separate the two?
Comment hidden (mozreview-request)
Assignee

Comment 7

2 years ago
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
Reporter

Comment 8

2 years ago
mozreview-review
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!
Attachment #8900110 - Flags: review?(sledru) → review+

Comment 9

2 years ago
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6803e175c0f3
extract clang format diff command generation into its own method r=sylvestre
https://hg.mozilla.org/mozilla-central/rev/6803e175c0f3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.