Issue370 encoding cookie #371
Conversation
Refactor the file-saving mechanism so that it's consistent between, eg, save and autosave Also slightly rework write_and_flush to reflect that it's being passed a file object, not a file descriptor (In principle the os.fsync ought to need the fileobj to call its .fileno() method, but it seems to be silently converting under the covers. I've confirmed, on Windows at least, that the correct underlying system API [FlushBuffersFile] is being called against the correct file path).
# Conflicts: # tests/test_logic.py
…the file being opened could not be decoded
|
This pull request introduces 1 alert when merging cd2b1ca into 8236b4b - view on lgtm.com new alerts:
Comment posted by lgtm.com |
|
Haven't got a chance yet to look into the code or the CI failures, but if you are printing or displaying any type of unicode on AppVeyor is likely you'll have to add before running the tests: |
|
Thanks, @carlosperate . I'd leave it for now; I'm still working through issues. I pushed it just so others could see the approach / progress. |
Add support for detecting newline convention
Plus fixing encoding stuff which didn't work previously!
…nit__.py Remove a debug trace from logic.py
mu/__init__.py
Outdated
|
|
||
| # Configure locale and language | ||
| # Define where the translation assets are to be found. | ||
| localedir = os.path.join('mu', 'locale') |
There was a problem hiding this comment.
This needs to be the same as what is currently (in master) in app.py:
localedir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'locale'))
|
Wow... this is great work. Please do say when it's ready for review etc... :-) |
|
So obviously this PR has sprawled somewhat. I've yet to devise tests (or to fix those which now fail because of the injection of the encoding cookie). I've raised a couple separate issues to highlight issues which I've attempted to address in the course of the PR Issue #380 -- standardising on \n for newlines internally to Mu while honouring the original data I still have unresolved questions from above, and I'm conscious that this has turned into a set of changes which are likely to have widespread impact, as they address loading & saving of files. I haven't, for example, considered the effect on embedded boards where we're packaging and sending code down the wire. My next step will be to add and fixup testing so we have a clean starting point for discussion, but please feel free to comment and/or challenge the approaches I've taken here. @carlosperate I'd like to hear from you especially on the line-ending change as I wasn't 100% sure of of the motivation behind your previous PR which used newline="" |
|
So testing just got a bit more complicated. Because I'm opening the file twice, once in binary, once in text mode, the current mock_open framework can't cope. (Took me about 20 mins to work out why my open(..., "rb") was not returning bytes! I remember discussing before the question of mocking, tho' I can't find the conversation anywhere. For now, I'm going to try for a filesystem-based test for a couple of tests, leaving the mocks in place for the Qt stuff. If that is gives a cleaner result, I'll move to leave it in. Let's see.. |
DO NOT MERGE
This is a PoC PR to test the approach advocated in #370
There are a number of unresolved issues, including:
(My answers in square brackets)
I've also added a "design note" under the new design folder in docs/. It's currently orphaned, but it's something I mooted a while ago and never got around to pursuing. If people like the idea, I'll add a design.rst to collect them together.