Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SpipAdapter #76

Open
camlafit opened this issue Dec 28, 2021 · 4 comments
Open

Add SpipAdapter #76

camlafit opened this issue Dec 28, 2021 · 4 comments

Comments

@camlafit
Copy link
Contributor

Hi

Could be nice to add SPIP cms

Thanks

camlafit added a commit to camlafit/cmsscanner that referenced this issue Dec 30, 2021
* Add SPIP adapter
* Detect by spip.php or spip_loader.php

Reference :
* CMS-Garden#76
@camlafit
Copy link
Contributor Author

Hello

I've a PR in progress at https://github.com/camlafit/cmsscanner/tree/issue-76_SPIPAdapter
But I've some questions before :

Thanks a lot

@SniperSister
Copy link
Member

I'm not sure looks as a good idea to run twice detectVersion()

Probably not the optimal way, but as you need the CMS version here, I can hardly see a way around it

about appendDetectionCriteria() we can have an other filespip_loader.php but If I add it, CMS could be detect twice.

If I understand you correctly, both files are present in SPIP instance, correct? If so, why adding a second file at all?

on detectSystem() we do a test to check files yet defined in appendDetectionCriteria() I don't understand why we duplicate this check ?

See:
https://github.com/camlafit/cmsscanner/blob/issue-76_SPIPAdapter/src/Command/DetectCommand.php#L121

All findings, regardless of their source adapter, are passed to all adapters for verification

@camlafit
Copy link
Contributor Author

camlafit commented Jan 4, 2022

Hello

I'm not sure looks as a good idea to run twice detectVersion()

Probably not the optimal way, but as you need the CMS version here, I can hardly see a way around it

Maybe detectVersion() could be set a local $version variable. Should be specific to this adapter but at least method will be run only once.

about appendDetectionCriteria() we can have an other filespip_loader.php but If I add it, CMS could be detect twice.

If I understand you correctly, both files are present in SPIP instance, correct? If so, why adding a second file at all?

Some user can personalize their spip.php to another name. Purpose is to catch this use case and use a workaround on an optional file specific to this CMS.

on detectSystem() we do a test to check files yet defined in appendDetectionCriteria() I don't understand why we duplicate this check ?

See: https://github.com/camlafit/cmsscanner/blob/issue-76_SPIPAdapter/src/Command/DetectCommand.php#L121
All findings, regardless of their source adapter, are passed to all adapters for verification

Ok I see, then actually this behavior is normal :)

@SniperSister
Copy link
Member

Maybe detectVersion() could be set a local $version variable. Should be specific to this adapter but at least method will be run only once.

If you go down that road, you'll class property storing a path => version map, as only one class instance is created and multiple CMS instances with multiple versions might by present on the filesystem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants