kossebau added inline comments.
INLINE COMMENTS
> carewolf wrote in script.cpp:316
> Sure, as I said, I only write it this way because a copy should be taken, and
> while Qt can work around declaring an async argument as a reference, I still
> consider it bad style to make that mistake.
>
>
carewolf added inline comments.
INLINE COMMENTS
> kossebau wrote in script.cpp:316
> With queued signals, any const-reference arguments are passed via an internal
> value-copy IIRC, so references are not out-dated.
> Can the same technique not be expected with any usages of invocables, like
>
kossebau added inline comments.
INLINE COMMENTS
> carewolf wrote in script.cpp:316
> Invokables can be called asynchronously (queued), if you take a reference and
> the isn't evaluated immediatly the reference might be invalid by the time the
> call is evaluated.
With queued signals, any
carewolf added inline comments.
INLINE COMMENTS
> kossebau wrote in script.cpp:316
> So why would it be preferable to have the value be copied there? In general,
> one prefers to avoid copies, so what is the different motvation here?
Invokables can be called asynchronously (queued), if you
kossebau added inline comments.
INLINE COMMENTS
> carewolf wrote in script.cpp:316
> Not really. I prefer to do it that way for invokable as the value will be
> copied, but const ref works too.
So why would it be preferable to have the value be copied there? In general,
one prefers to avoid
carewolf added inline comments.
INLINE COMMENTS
> kossebau wrote in script.cpp:316
> Hi @carewolf . Curious: Why all the non-const-ref QString argument types here?
>
> Is there a need to do that with Q_INVOKABLE methods somehow?
> Asking because clazy falgs these, compare D25039
>
kossebau added inline comments.
INLINE COMMENTS
> script.cpp:316
> // @returns true if @p str matches the shell @p pattern
> -QScriptValue ShExpMatch(QScriptContext *context, QScriptEngine *engine)
> +Q_INVOKABLE QJSValue ShExpMatch(QString str, QString patternStr)
> {
Hi @carewolf . Curious:
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:4ad1902278f0: Port kpac from QtScript (authored by
carewolf).
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D23801?vs=65690=65741
REVISION DETAIL
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
I'll trust your expertise with such ports.
I wouldn't know how to test this either.
REPOSITORY
R241 KIO
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D23801
To:
knauss added a task: T11530: Investigate removal of KIO's KPAC in favor of
QNetworkProxy.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D23801
To: carewolf, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
carewolf created this revision.
carewolf added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
carewolf requested review of this revision.
REVISION SUMMARY
Migrate the QtScript code to QJS classes.
Warning: untested
REPOSITORY
11 matches
Mail list logo