Feature #34922

Allow .ts file extension for static typoscript templates

Added by Sebastian Michaelsen over 7 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Should have
Category:
-
Target version:
Start date:
2012-03-16
Due date:
% Done:

100%

PHP Version:
Tags:
Complexity:
medium
Sprint Focus:

Description

At the moment the following static typoscript filenames are allowed:

setup.txt
constants.txt
include_static.txt
include_static_files.txt

My intention is to also allow ".ts" as file extensions because they're commonly used and IDEs can recognize those file as TypoScript.

I was already playing around with the code and what makes me worry is performance. It's quite expensive to check all the allowed filenames and read them.

I benchmarked different situations (folder with only .txt / only .ts / both / none) and in average it slows down the reading of static templates by about 48%. I don't know if this is acceptable in change for the convenience you get.
Maybe there's a possibility for a smart caching solution or other ideas on improving the performance. Ideas are welcome.

I will add my best effort patch here shortly.

34922.diff View (4.96 KB) Sebastian Michaelsen, 2012-03-16 13:44

Associated revisions

Revision f60f1209 (diff)
Added by Sebastian Michaelsen about 7 years ago

[FEATURE] Allow .ts file extension for static typoscript templates

At the moment the following static typoscript filenames are allowed:

setup.txt
constants.txt
include_static.txt
include_static_files.txt

  • Allow ".ts" as file extensions
  • Allow mixed usage of .ts and .txt
  • .ts precedes .txt

Change-Id: I0ffd9ef50a07dfbaa8388d525c5ced09d5070103
Fixes: #34922
Releases: 4.8
Reviewed-on: http://review.typo3.org/9736
Reviewed-by: Philipp Gampe
Reviewed-by: Stefan Neufeind
Reviewed-by: Simon Schaufelberger
Tested-by: Simon Schaufelberger
Reviewed-by: Susanne Moog
Tested-by: Susanne Moog

Revision 690b7d0c (diff)
Added by Pierrick Caillon over 4 years ago

[FEATURE] Allow .ts file extension for static typoscript templates

Static TypoScript support alternative primary names for constants and
setup file:
  • constants.ts
  • setup.ts
Old names are still working:
  • constants.txt
  • setup.txt

Resolves: #34922
Releases: master
Change-Id: I6868c22c95337812e6946f16655f024f2ed80471
Reviewed-on: http://review.typo3.org/40264
Reviewed-by: Jan Helke <>
Tested-by: Jan Helke <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

History

#1 Updated by Sebastian Michaelsen over 7 years ago

This is my first patch.

  • Mixed usage of .txt and .ts is allowed.
  • .ts precedes .txt

#2 Updated by Sebastian Michaelsen over 7 years ago

One more point on performance:

Maybe it's not as critical as 48% sounds. According to my measurements reading out the folder is in a time scale of 0,2ms up to 1,2ms. So in the worst case you will lose less than 1ms per extension template. It's still worth thinking about optimization, but not a blocker I think.

#3 Updated by Oliver Hader over 7 years ago

Sounds good. Once you're done with your patch, could you please add it to review.typo3.org so others can test and review it?
See http://wiki.typo3.org/Contribution_Walkthrough_Tutorials for accordant information.

#4 Updated by Gerrit Code Review over 7 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9736

#5 Updated by Gerrit Code Review about 7 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9736

#6 Updated by Sebastian Michaelsen about 7 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#7 Updated by Christian Kuhn about 7 years ago

  • Status changed from Resolved to New
  • TYPO3 Version changed from 4.7 to 6.0

The merge had to be reverted again :(

It seems the ts files where now in different order, and we were unable to fix this quickly.

#8 Updated by Joh. Feustel about 7 years ago

Also this breaks if there are includes of not loaded extensions in a sys_template record: t3lib_extMgm::isLoaded($ISF_extKey) has been removed but t3lib_extMgm::extPath($extensionKey) only works for loaded extensions.

#9 Updated by Philipp Gampe about 7 years ago

Ah, that was the reason ;)

#10 Updated by Simon Schaufelberger almost 7 years ago

is somebody willing to make a better patch? that would be great!

#11 Updated by Stefan Neufeind almost 6 years ago

Would be great to have ...

#12 Updated by Mathias Schreiber over 4 years ago

  • Target version set to 7.2 (Frontend)

#13 Updated by Benni Mack over 4 years ago

  • Target version changed from 7.2 (Frontend) to 7.4 (Backend)
  • Sprint Focus set to On Location Sprint

#14 Updated by Pierrick Caillon over 4 years ago

  • Status changed from New to In Progress
  • Assignee set to Pierrick Caillon

#15 Updated by Pierrick Caillon over 4 years ago

I think the include_static and include_static_file files should not accept the "ts" extension as they are list of database references and folders respectively.

#16 Updated by Gerrit Code Review over 4 years ago

  • Status changed from In Progress to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40264

#17 Updated by Gerrit Code Review over 4 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40264

#18 Updated by Gerrit Code Review over 4 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40264

#19 Updated by Gerrit Code Review over 4 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40264

#20 Updated by Gerrit Code Review over 4 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40264

#21 Updated by Pierrick Caillon over 4 years ago

  • Status changed from Under Review to Resolved

#22 Updated by Mathias Schreiber about 4 years ago

  • Sprint Focus deleted (On Location Sprint)

#23 Updated by Simon Schaufelberger about 4 years ago

let's drink a beer on this! yeah!!!

#24 Updated by Riccardo De Contardi almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF