quark added a comment.
In https://phab.mercurial-scm.org/D472#9064, @martinvonz wrote:
> I failed with the update of this patch, so see
https://phab.mercurial-scm.org/D563 instead
I guess it's because of the metaedit crash. This can be updated if you have
`Differential Revision:
martinvonz added a comment.
I failed with the update of this patch, so see
https://phab.mercurial-scm.org/D563 instead
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D472
To: martinvonz, #hg-reviewers, quark, mjpieters
Cc: yuja, lothiraldan, quark, mjpieters,
yuja added a comment.
This is off-topic, but if `wrapfunction()` is a context manager, I think
`unwrapfunction()` could also be a context manager.
INLINE COMMENTS
> quark wrote in extensions.py:424
> I think using the return value of wrapfunction could lead to hidden bugs:
>
> oldfunc =
quark added inline comments.
INLINE COMMENTS
> yuja wrote in extensions.py:424
> I see, but I don't think it's more common than wrapping a plain
> function.
>
> If the compatibility matters, we probably shouldn't replace
> `wrapfunction` with a context manager.
I think using the return value
yuja added inline comments.
INLINE COMMENTS
> martinvonz wrote in extensions.py:424
> See commit message. It's for compatibility with existing callers that bind
> the returned value.
I see, but I don't think it's more common than wrapping a plain
function.
If the compatibility matters, we
yuja added inline comments.
INLINE COMMENTS
> extensions.py:424
> +
> +def __get__(self, instance, owner):
> +util.nouideprecwarn("Using the return value of wrapfunction() as a "
When will this `__get__()` be called?
`wrapfunction()` is generally used for wrapping a free function,
mjpieters accepted this revision.
mjpieters added a comment.
I was being too strict; like `open()` it's sometimes fine to apply the action
in the constructor.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D472
To: martinvonz, #hg-reviewers, quark, mjpieters
martinvonz added a comment.
In https://phab.mercurial-scm.org/D472#7758, @quark wrote:
> In https://phab.mercurial-scm.org/D472#7756, @lothiraldan wrote:
>
> > All users of wrapfunction will need to be updated anyway to use the
context manager style
>
>
> I think only those who
quark added a comment.
In https://phab.mercurial-scm.org/D472#7756, @lothiraldan wrote:
> All users of wrapfunction will need to be updated anyway to use the context
manager style
I think only those who use the return value of wrapfunction will need change
(actually, new code is
lothiraldan added a comment.
Maybe instead of replacing wrapfunction with a context manager, what about
introducing it with a separate name? All users of wrapfunction will need to be
updated anyway to use the context manager style
REPOSITORY
rHG Mercurial
REVISION DETAIL
quark added inline comments.
INLINE COMMENTS
> mjpieters wrote in extensions.py:408-412
> Move this to `__enter__`. You want to be able to create the context manager
> separately from actually entering the context:
>
> cmwrapper = wrapfunction(cont, 'name', wrapper)
>
> # do other
mjpieters added inline comments.
INLINE COMMENTS
> extensions.py:408-412
> +origfn = getattr(container, funcname)
> +assert callable(origfn)
> +wrap = bind(wrapper, origfn)
> +_updatewrapper(wrap, origfn, wrapper)
> +setattr(container, funcname, wrap)
quark added subscribers: mjpieters, quark.
quark accepted this revision.
quark added a comment.
cc python expert @mjpieters to see if he has additional comments. But I think
this is good enough.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D472
To:
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
Several extensions exist that temporarily want to wrap a function (at
least narrowhg, any many of the extensions in hg-experimental). That's
why we have the
14 matches
Mail list logo
