Two more ways to change output method temporarily #220
Conversation
plq
changed the title
plq
changed the title
src/lxml/serializer.pxi
Outdated
| @@ -812,6 +812,15 @@ cdef class _IncrementalFileWriter: | |||
| tree.xmlOutputBufferFlush(self._c_out) | |||
| self._handle_error(self._c_out.error) | |||
|
|
|||
| def method(self, method): | |||
| """element(self, tag, attrib=None, nsmap=None, **_extra) | |||
| def __cinit__(self, _IncrementalFileWriter writer not None, int method): | ||
| self._writer = writer | ||
| self._new_method = method | ||
| self._old_method = writer._method |
There was a problem hiding this comment.
It's not correct to remember the original value here. Context manager objects can be reused and there is no guarantee that users create this object directly as part of a with-statement, so the exit method might store back an incorrect value.
Either reject an unexpected current method value when entering/exiting to prevent reuse, or use a stack for storing away the old values. Preventing reuse seems simple and acceptable, though (and would obviously need testing :-) ).
There was a problem hiding this comment.
I guess I'm among the guiltiest when it comes to using context managers outside of with statement. Look for __enter__ and __exit__ cals in: https://github.com/arskom/spyne/blob/f92823a512eb8be71adb24fa151b397f27df5b9f/spyne/protocol/cloth/to_cloth.py#L278
To me the contract with using context managers' private apis is it's user's responsibility to enter/exit in correct order. I could totally take the precautions you mention but aren't they needless overhead for people who use the apis the correct way (within the with statement) or incorrect way (outside the with statement) but who are supposed to be people who know what they are doing?
Maybe in the spirit of protecting the expert, we could set a flag on the context manager to prevent it from being __exit__'ed twice.
However if you insist, I will implement the stack.
There was a problem hiding this comment.
I actually don't think that we need a stack here. It should be enough to check that the output method didn't change between the end of __enter()__ and the start of __exit()__, i.e. to save the old method on entry and check that the current method is still new_method on exit. If it's not, it's a bug in the calling code and the user should know about it, i.e. get an exception. Oh, and maybe prevent re-entry before exiting by checking that old_method is set to an invalid value on entry. That would detect programming problems while not restricting users too much.
There was a problem hiding this comment.
It should be enough to check that the output method didn't change between the end of enter() and the start of exit()
done.
Oh, and maybe prevent re-entry before exiting by checking that old_method is set to an invalid value on entry.
not like that, but done.
… a context manager without having to write an element
plq
force-pushed
the
method-ctxmanager
branch
2 times, most recently
from
34a4c25
to
9043bb6
Compare
plq
force-pushed
the
method-ctxmanager
branch
from
9043bb6
to
0bfc0d9
Compare
|
Please let me know if you need anything else |
# Conflicts: # src/lxml/serializer.pxi
|
thanks a lot! |
Another way of changing output method temporarily.