[QGIS-Developer] [qgis/QGIS] Allow setting a custom path pre-processor for QgsPathResolver (#30842)

Raymond Nijssen r.nijssen at terglobo.nl
Mon Aug 19 23:48:52 PDT 2019


Hi Nyall,

That sounds exactly what we need!

In this case I would think addPathPreprocessor() would be a better name 
than setPathPreprocessor().

The chain order should not matter since every preprocessor will check 
for something specific, in our case paths with our plugin name.

Kind regards,
Raymond

Terglobo
Fahrenheitstraat 1
5223 BJ 's-Hertogenbosch
The Netherlands
+31 (0) 6 25 31 49 83

On 20-08-19 08:33, Nyall Dawson wrote:
> On Fri, 16 Aug 2019 at 22:24, Raymond Nijssen <r.nijssen at terglobo.nl> wrote:
>>
>> Hi Nyall and others here,
>>
>> I'm writing here to the dev list because the PR has already been pulled
>> and closed.
>>
>> We implemented this new code in our plugin and it works very well! One
>> thing is bothering me though, and I hope I am wrong here.
>>
>> As I wanted to do a clean up in the plugin unload method, I figured I
>> can never remove the preprocessor. Since our plugin overwrites the
>> pathPreprocessor, it also overwrites any existing preprocessor. Or even
>> worse, another plugin could overwrite ours! :)
>>
>> I think the most elegant solution is a list of pathPreprocessors that
>> will return either the input path or an edited one (like now). The path
>> resolver would run them all in a chain and fix all paths for all
>> plugins. By giving them an ID, it would also be possible to remove them
>> (or overwrite them with a processor that always returns the input) on
>> unloading the plugin.
> 
> Right. I debated this a lot internally when writing that API hook, and
> came to the conclusion that it's not really intended for use in public
> plugins, but rather just for those "internal only" organisation
> plugins. I guess this was the wrong choice :P
> 
> I'd propose we could adjust the api:
> 
>      static void setPathPreprocessor( const std::function< QString(
> const QString &filename )> &processor );
> 
> to
> 
>     static QString setPathPreprocessor( const std::function< QString(
> const QString &filename )> &processor );
> 
> which returns an automatically generated unique string identifying the
> processor.
> And then we add new call
> 
>     static void removePathPreprocessor( const QString& id );
> 
> Internally, the processors would just be called in the order they've
> been set, chaining the result from one into the next.
> 
> How's that sound?
> Nyall
> 
> 
> 
> 
>> I guess this is quite some work for a not much used feature (yet) and
>> maybe the reason you chose to implement it this less comlplicated way.
>> But it is potentially really hard to debug problems if 2 or more plugins
>> collide on this and we want to reproduce the error. We would need to
>> know all the plugins and versions our users have installed and look into
>> that code.
>>
>> Am I right? Please let me know your thoughts about this.
>>
>> Kind regards,
>> Raymond
>>
>>
>>
>> On 25-07-19 00:41, Nyall Dawson wrote:
>>> Merged #30842 <https://github.com/qgis/QGIS/pull/30842> into master.
>>>
>>>>>> You are receiving this because you commented.
>>> Reply to this email directly, view it on GitHub
>>> <https://github.com/qgis/QGIS/pull/30842?email_source=notifications&email_token=AAFGO63EK64SG3XXAUAIAVTQBDLC3A5CNFSM4IFT6T2KYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOSV5I2GQ#event-2507836698>,
>>> or mute the thread
>>> <https://github.com/notifications/unsubscribe-auth/AAFGO63PMWSOTDPTH365GSLQBDLC3ANCNFSM4IFT6T2A>.
>>>
>> _______________________________________________
>> QGIS-Developer mailing list
>> QGIS-Developer at lists.osgeo.org
>> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> 


More information about the QGIS-Developer mailing list