jweiland-net / video_shariff

This extension provides more privacy when embedding videos in frontend.
https://extensions.typo3.org/extension/video_shariff
Other
5 stars 6 forks source link

1.6.0 is breaking because of renamed setup.txt > setup.typoscript #22

Closed t3easy closed 4 years ago

froemken commented 4 years ago

Which TYPO3 Version are you using? The new .typoscript extension is available since TYPO3 8.7.: https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/8.7.x/Feature-78161-IntroduceTypoScriptFileExtension.html. So it may not work in TYPO3 8.7.0.

t3easy commented 4 years ago

Ah, the description was lost... It is breaking if you include the file with

or @include
froemken commented 4 years ago

Ah, you mean because of the renamed file which is breaking in this feature release (1.6.0)?

t3easy commented 4 years ago

Yes. Releasing a 2.0.0 would have been good to follow semver rules.

pascal20997 commented 4 years ago

I think we should create a new bugfix or feature release and add a setup.txt that includes the setup.typoscript file. Maybe we can add a comment into that file like "This file will be removed in video_shariff 3.0.0." The TemplateService searches for .typoscript first, so the setup.txt shouldn't make problems in addition to the .typoscript file. What do you think?

t3easy commented 4 years ago

I don't know, what happens in a case if both .txt and .typoscript exist but I think .typoscript has priority and the txt is not used if it's included by include_static_file. The added required constants.typoscript is also "breaking" if files are loaded with INCLUDE_TYPOSCRIPT, because it's new that you have to include them...

pascal20997 commented 4 years ago

You're right that's a new requirement. One solution would be either to increase the version to 2.0.0 or to customize the setup.txt a bit. There is only one constant right now so we could override the value of the default thumbnail inside the setup.txt.

Example of setup.txt:

# This file was replaced by setup.typoscript.  It will be removed in video_shariff 2.0.0

<INCLUDE_TYPOSCRIPT: source="FILE:EXT:video_shariff/Configuration/TypoScript/setup.typoscript">
lib.video_shariff.defaultThumbnail = EXT:video_shariff/Resources/Public/Images/DefaultThumbnail.png
t3easy commented 4 years ago

By the way, configuring settings of a plugin in the lib tree is odd ;) plugin.videoshariff.settings.defaultThumbnail corresponds more to the convention of TYPO3 Extensions.

pascal20997 commented 4 years ago

The idea of lib.video_shariff was that video_shariff is not a "plugin" like e.g. news. It just overrides the fluid_styled_content templates.