Feedback to: Textpattern CMS 4.6.0 beta released

philwareham · 2016-06-13 15:18:41

Bloke wrote #299679:

Implementing custom.whatever files per theme will work fine, but the core must not use it.

So my current proposal is fine then (see latest commits)? All I have done in essence is consolidate several custom css snippets into one file.

Bloke · 2016-06-13 15:48:19

philwareham wrote #299681:

So my current proposal is fine then (see latest commits)?

Sadly no :-(

The fact you’ve created custom.css inside both themes means:

Someone sets the admin_custom_css trigger.
They change custom.css to their liking.
Customisation result!
We release 4.6.1, or whatever.
They upgrade and lose their edits to custom.css because core steamrollers over the top of it.
Customisation fail :-(

Bottom line: if we introduce such a mechanism, core can only call the custom file(s) if the trigger(s) are defined, it cannot actually use and package the custom files up in the download itself.

Does that make sense?

philwareham · 2016-06-13 15:51:10

Better proposal welcomed…

Bloke · 2016-06-13 15:59:43

philwareham wrote #299684:

Better proposal welcomed…

Tweak it akin to the way it’s done in line 97 of the root index.php?

Basically, instead of hard-coding custom.css in the theme’s php file, allow the admin_custom_css entry in config.php to define a filename. e.g. guguser might set:

define('admin_custom_css', 'gug_cust.css');

and then your theme dutifully detects this and loads theme/css/{name-of-defined-file}.

Or something like that.

philwareham · 2016-06-13 16:05:55

I don’t see how that would be any different from what I have – it’s just as likely to get blown away during an update if someone uploads the directory instead of just the individual files. I need to rethink the whole custom thing really.

philwareham · 2016-06-13 17:07:19

I guess I’ll have to revert for now, sorry :(

Bloke · 2016-06-13 17:17:00

philwareham wrote #299686:

it’s just as likely to get blown away during an update if someone uploads the directory instead of just the individual files.

Not unless you delete the file first. Uploading a directory full of files merges the content with what’s there already, overwriting any that clash. If a person’s chosen filename is not in the core package, it can’t get overwritten and will persist an upgrade.

The only time they’ll have trouble is if they name their custom file(s) the same as one of the core files. But hey, if you do that, it’s the same as modding a core file, so you get what’s coming to ya!

As long as the file name they choose to hold the customisations isn’t the same name as one in core, all will be good. Trust me, I’m a doctor.

Bloke · 2016-06-13 17:20:03

philwareham wrote #299687:

I guess I’ll have to revert for now, sorry :(

Not necessary. It’ll work fine as it is. Just means we need to have an additional line to load the custom script as well as custom.css, which you can maybe rename as core_extensions.css or something else if we feel that custom.css might imply that it’s user-customisable, when it actually isn’t.

EDIT: Hmm, hang on. Maybe it’s better to have this file as an example of what one could do to customise it, and have it available in a well-known download location as a support file. Just not include it in the download, so someone can use it as a basis if they want? Or create their own.

phiw13 · 2016-06-13 22:44:13

Bloke wrote #299689:

Bloke, first thanks for explaining the issue. As far as I understand it, the basic mechanism of what Phil has implemented is fine, with one exception: the file name.

EDIT: Hmm, hang on. Maybe it’s better to have this file as an example of what one could do to customise it, and have it available in a well-known download location as a support file. Just not include it in the download, so someone can use it as a basis if they want? Or create their own.

There. Why not include Phil’s file as file-name-example.css in the core download. If the end user wants to use it, it needs to be renamed to file-name.css (in the same location/folder). Or they create their own, equally named file-name.css. That would be similar to what happens with config.php. No issues here at update time, whether using sFTP or GIT or other mechanism to update.

Bloke · 2016-06-14 07:45:26

phiw13 wrote #299693:

As far as I understand it, the basic mechanism of what Phil has implemented is fine, with one exception: the file name.

Correct.

Why not include Phil’s file as file-name-example.css in the core download.

Perfect. I like the way it’s all been consolidated into a single file: much neater. I was just going to rename the file and tweak the bit that reads the filename, but Phil’s reverted the changes already. Dammit. I wonder if it’s possible to un-revert reverted code? :-)

phiw13 · 2016-06-14 07:56:14

Bloke wrote #299701:

Dammit. I wonder if it’s possible to un-revert reverted code? :-)

I hope so!

Fwiw, I’ve updated Sandpaces 4.6b with the above, naming the file “custom/custom.css”.

The only drawback (minor, I think) is it forces the person customising the theme to use a predefined filename. If we settle down on a name for core and third-party themes to use it could even be a plus point, if the need arises for some obscure security reasons to lock this down. Probably this would be more of a concern if a similar mechanism is added for JS files is added, as suggested by Ruud.

Bloke · 2016-06-14 08:16:28

phiw13 wrote #299702:

The only drawback (minor, I think) is it forces the person customising the theme to use a predefined filename.

Yes, that’s why I was going to allow it to be configured. The way I see it, only the admin should have control over config.php so if someone wants to shoot themselves in a toe-based appendage by specifying a file they don’t trust, it’s up to them. We don’t sanitise the pre_publish_script file location in any way either, for the same reason: caveat utilitor.

If we settle down on a name for core and third-party themes to use it could even be a plus point

I’m happy to use a fixed filename. It potentially make things easier for theme tweakers as they don’t need to remember to set the filename up in config.php. If we renamed the one in the distro as an example filename, the core could always check for custom.css (or whatever name we choose) in the theme. If the file exists, use it. If not, no sweat. Same could be done a for well-known .js file.

Textpattern CMS

Textpattern CMS support forum

#61 2016-06-13 15:18:41

Re: Feedback to: Textpattern CMS 4.6.0 beta released

Bloke wrote #299679:

#62 2016-06-13 15:48:19

Re: Feedback to: Textpattern CMS 4.6.0 beta released

philwareham wrote #299681:

#63 2016-06-13 15:51:10

Re: Feedback to: Textpattern CMS 4.6.0 beta released

#64 2016-06-13 15:59:43

Re: Feedback to: Textpattern CMS 4.6.0 beta released

philwareham wrote #299684:

#65 2016-06-13 16:05:55

Re: Feedback to: Textpattern CMS 4.6.0 beta released

#66 2016-06-13 17:07:19

Re: Feedback to: Textpattern CMS 4.6.0 beta released

#67 2016-06-13 17:17:00

Re: Feedback to: Textpattern CMS 4.6.0 beta released

philwareham wrote #299686:

#68 2016-06-13 17:20:03

Re: Feedback to: Textpattern CMS 4.6.0 beta released

philwareham wrote #299687:

#69 2016-06-13 22:44:13

Re: Feedback to: Textpattern CMS 4.6.0 beta released

Bloke wrote #299689:

#70 2016-06-14 07:45:26

Re: Feedback to: Textpattern CMS 4.6.0 beta released

phiw13 wrote #299693:

#71 2016-06-14 07:56:14

Re: Feedback to: Textpattern CMS 4.6.0 beta released

Bloke wrote #299701:

#72 2016-06-14 08:16:28

Re: Feedback to: Textpattern CMS 4.6.0 beta released

phiw13 wrote #299702:

Board footer