Summary: | check-webkit-style should restore the original file name if it is used from Emacs's flymake so that it can get an correct header guard name | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hayato Ito <hayato> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Hayato Ito
2010-03-24 19:24:07 PDT
Created attachment 51596 [details]
fix-header-guard-name-for-flymake
Comment on attachment 51596 [details] fix-header-guard-name-for-flymake > - emacs_flymake_suffix = '_flymake.cpp' > - if abs_filename.endswith(emacs_flymake_suffix): > - abs_filename = abs_filename[:-len(emacs_flymake_suffix)] + '.cpp' > + abs_filename = re.sub(r'_flymake\.cpp$', '.cpp', abs_filename) Let me confirm, is this just a refactoring? > + # Special case for flymake > + error_collector = ErrorCollector(self.assert_) > + self.process_file_data('mydir/Foo_flymake.h', 'h', [], error_collector) > + self.assertEquals( > + 1, > + error_collector.result_list().count( > + 'No #ifndef header guard found, suggested CPP variable is: %s' > + ' [build/header_guard] [5]' % expected_guard), > + error_collector.result_list()) > + I guess what we want to ensure is "#ifndef Foo_h_" won't be warned when it is put in "Foo_flymake.h" ? If so, please add a test case for this case. By the way, I think putting your elisp to use check-webkit-style with flymake may help someone. Could you show it? Created attachment 51609 [details]
fix-header-guard-name-for-flymake
Thank you for the review. (In reply to comment #2) > (From update of attachment 51596 [details]) > > - emacs_flymake_suffix = '_flymake.cpp' > > - if abs_filename.endswith(emacs_flymake_suffix): > > - abs_filename = abs_filename[:-len(emacs_flymake_suffix)] + '.cpp' > > + abs_filename = re.sub(r'_flymake\.cpp$', '.cpp', abs_filename) > > Let me confirm, is this just a refactoring? Yes, it is just a refactoring. > > + # Special case for flymake > > + error_collector = ErrorCollector(self.assert_) > > + self.process_file_data('mydir/Foo_flymake.h', 'h', [], error_collector) > > + self.assertEquals( > > + 1, > > + error_collector.result_list().count( > > + 'No #ifndef header guard found, suggested CPP variable is: %s' > > + ' [build/header_guard] [5]' % expected_guard), > > + error_collector.result_list()) > > + > > I guess what we want to ensure is "#ifndef Foo_h_" won't be warned when it is > put in "Foo_flymake.h" ? If so, please add a test case for this case. I've add a test case. Thank you! > By the way, I think putting your elisp to use check-webkit-style with flymake > may help someone. Could you show it? Sure. It should be something like: (require 'flymake) (defun flymake-check-webkit-style-init () (list "check-webkit-style" (list (flymake-init-create-temp-buffer-copy 'flymake-create-temp-inplace)))) (add-to-list 'flymake-allowed-file-name-masks '("WebKit/.*\\.cpp\\'" flymake-check-webkit-style-init)) (add-to-list 'flymake-allowed-file-name-masks '("WebKit/.*\\.h\\'" flymake-check-webkit-style-init)) After evaluating the above elisp, just doing M-x flymake. Comment on attachment 51609 [details]
fix-header-guard-name-for-flymake
Looks good! Thanks for adding the test and showing your elisp-fu.
Comment on attachment 51609 [details] fix-header-guard-name-for-flymake Clearing flags on attachment: 51609 Committed r56763: <http://trac.webkit.org/changeset/56763> All reviewed patches have been landed. Closing bug. |