Bug 63419

Summary: Add encoding parameter to filesystem.py functions
Product: WebKit Reporter: Roland Steiner <rolandsteiner>
Component: Tools / TestsAssignee: Roland Steiner <rolandsteiner>
Status: RESOLVED WONTFIX    
Severity: Normal CC: dominicc, dpranke, morrita, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 63514    
Bug Blocks: 61772    
Attachments:
Description Flags
Patch dpranke: review-

Roland Steiner
Reported 2011-06-26 22:49:56 PDT
Currently, functions to open/read files largely assume 'utf8' as the encoding to use. This should be override-able by the caller.
Attachments
Patch (9.48 KB, patch)
2011-06-27 00:02 PDT, Roland Steiner
dpranke: review-
Roland Steiner
Comment 1 2011-06-27 00:02:32 PDT
Dominic Cooney
Comment 2 2011-06-27 06:02:21 PDT
Comment on attachment 98668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98668&action=review > Tools/Scripts/webkitpy/common/system/filesystem.py:-199 > - def open_text_file_for_writing(self, path, append=False): There were no callsites hitherto using append? > Tools/Scripts/webkitpy/common/system/filesystem_unittest.py:178 > + fs.write_text_file(text_path, unicode_text_string, encoding='Windows-1252') Python 2.7 documentation seems to indicate that it is little-w windows… how do you know your encoding is working?
Tony Chang
Comment 3 2011-06-27 11:04:23 PDT
Out of curiosity, what will you be using this for? I just wonder if it would be simpler (less code) for you to manually do the encoding conversion and use read_binary_file/write_binary_file.
Dirk Pranke
Comment 4 2011-06-27 13:20:31 PDT
Comment on attachment 98668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98668&action=review These interfaces were intentionally designed so that the caller did not have to worry about the encoding, and that if if a file was being passed to the text versions of these routines, we could be fairly certain it was stored on disk as utf-8. I do not want to lose those principles lightly. Until there's multiple callers that need this functionality, I would prefer it if you just used the read/write_binary_file routines and did the encoding/decoding in the calling routine instead (like Tony suggests). Is that okay? >> Tools/Scripts/webkitpy/common/system/filesystem.py:-199 >> - def open_text_file_for_writing(self, path, append=False): > > There were no callsites hitherto using append? I think that that is correct. I think I added it at one point and ended up not needing it.
Roland Steiner
Comment 5 2011-06-28 00:15:54 PDT
(In reply to comment #2) > > Tools/Scripts/webkitpy/common/system/filesystem_unittest.py:178 > > + fs.write_text_file(text_path, unicode_text_string, encoding='Windows-1252') > > Python 2.7 documentation seems to indicate that it is little-w windows… how do you know your encoding is working? Comparison to the expected binary representation seems to work (after the issues from https://bugs.webkit.org/show_bug.cgi?id=63514 have been corrected). (In reply to comment #3) > Out of curiosity, what will you be using this for? I just wonder if it would be simpler (less code) for you to manually do the encoding conversion and use read_binary_file/write_binary_file. This is meant to read/write Visual Studion project files for https://bugs.webkit.org/show_bug.cgi?id=61772 which are encoded in "Windows-1252" (using an upper-case "W" ^_-) (In reply to comment #4) > (From update of attachment 98668 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=98668&action=review > > These interfaces were intentionally designed so that the caller did not have to worry about the encoding, and that if if a file was being passed to the text versions of these routines, we could be fairly certain it was stored on disk as utf-8. I do not want to lose those principles lightly. > > Until there's multiple callers that need this functionality, I would prefer it if you just used the read/write_binary_file routines and did the encoding/decoding in the calling routine instead (like Tony suggests). Is that okay? Sure can do, although I don't see the drawback here. > >> Tools/Scripts/webkitpy/common/system/filesystem.py:-199 > >> - def open_text_file_for_writing(self, path, append=False): > > > > There were no callsites hitherto using append? > > I think that that is correct. I think I added it at one point and ended up not needing it. Yes, I checked all call-sites to make sure it wasn't used, but forgot to document this part of the change.
Note You need to log in before you can comment on or make changes to this bug.