Twig Function Semantic Naming

[hanakin]

This is a discussion for why we need to change the convention that we are using for naming any back-end functions that are to be used in the template. https://tracker.phpbb.com/browse/PHPBB3-16244

Currently we are using the twig convention for naming all of these which is one_two_three

Going forward in 4.0 one of our goals is to completely separate all concerns which means the js does not directly touch or inject any html/css, the php code does not inject any html/css/js

This is considered a best practice to ensure a clean organized codebase as well as improving legibility and control of all aspects of the app.

Yet another of our goals related to this is to improve the clarity and confidence of our front-end code.

Which you can read more about here for how we are accomplishing this all with the CSS: https://area51.phpbb.com/docs/dev/devel ... amespacing

BLUF:

• Clarity:
  • How much information can we glean from the smallest possible source?
  • Is our code self-documenting?
  • Can we make safe assumptions from a single context?
  • How much do we have to rely on external or supplementary information in order to learn about a system?
• Confidence:
  • Do we have enough knowledge about a system to be able to safely interface with it?
  • Do we know enough about our code to be able to confidently make changes?
  • Do we have a way of knowing the potential side effects of making a change?
  • Do we have a way of knowing what we might be able to remove?

This also enables better find & search capabilities for the code base as well.

With all of this in mind the issue is with the naming of the twig functions as they do not meet all the aspects of clarity and confidence . It may not be possible to meet all of these but we should strive to meet as many as possible whenever building the front-end of any highly customizable or extendable application.

I am open to feedback on exactly how to accomplish this but some of the suggested ideas are as follows.

• Use PascalCase for the functions as this breaks standards for all front-end languages. However it is a major deviation from our local established standards
• Append twig_ for all the functions. This one is the most specific and descriptive and follows the same guidelines as the approach used for CSS.
• Similar to the above we could use an appended flag that more closely follows that of the CSS approach like f_ for function. This has the added benefit over the above of being short but also using a flag for the definition to make it more obvious that the first part of the name is not part of the name.

Again this is not an all encompassing list of but just a few to get things started.

Keep in mind that this would require changes to all current twig functions to meet whatever new standard we enforce. EXPS: JumpBox, twig_jumpbox, f_jumpbox, Lang, twig_lang, f_lang

[david63]

Personally I cannot see a problem with any naming convention - just as long as it is adhered to and consistent.

But should this discussion not be more wide ranging? phpBB has, since its inception, insisted on snake_case naming convention for functions/variables. Today snake_case is not widely used and in fact none of the add ins that phpBB now uses use that convention - and in fact phpBB's .css and .js files have not used it for some time (if ever)

I appreciate that this is deviating from the point of this topic but can we not move towards a consistent convention in all aspects of phpBB and at the same time become more aligned with "industry standards"?

[hanakin]

Snake case is currently only phpbbs standard for php. The js and css standards changed in 3.1

[MattF]

I disagree with making any of these changes to the way we currently name our custom twig functions, just for the purposes of search/find the code base. I mean, search for lang( within the styles directory and you'll find what you're looking for. As somebody who has been searching phpBB's codebase hundreds of thousands of times for over a decade I've never thought of changing function names.

[3Di]

And I confirm what I wrote in the tracker, I too am in total disagreement with all these changes.

And then why on earth after more than 16 years of exploring this codebase should I (and many many others) change my established habits?
Any decent editor allows you to search for files with a certain extension within certains folders/sub-folders, such as .html, .php, .yaml etc. - There would also be no need to add a parenthesis to the name.

As if that weren't enough, the already well known preparation for the complete incompatibility with the previouses phpBB, as can be seen from the discussion.

If I can afford to say so, I would concentrate on other matters far more important than these. IMHO.

[hanakin]

Both response focus on just the find and search ability. Both fail to provide any arguments against any of the other valid points I laid out. Either-way as I mentioned the front-end codebase is all going to follow strict guidelines in 4.0 to answer all of this which is a major release so this is the time to discuss this. Just because something works one way in one version does not mean that it has to work that way in the next. I do not see that as a valid counter argument alone vs all the other benefits I have laid out.

You are providing arguments from a backend development/ext development perspective. But nothing from a front-end perspective, style development, admin, end-user perspective. So you have to look at it from those perspectives as well. Especially since its use-case is in the front-end code which is the only thing the majority ever sees or touches.

How would you know to search for lang( if you are unfamiliar with the codebase? How would you know that its a twig function without knowledge of twig/php?

You would not. Yes you could document every function but there is still overlap and naming confusions. Having a scoped name provides a simple frame of reference for all of these rather than having to learn all of them when working on style development.

Here is a really good article to help further outline the importance https://csswizardry.com/2015/03/more-tr ... amespaces/

[mrgoldy]

I remember when I first started coding, reading camelCase and PascalCase was really confusing to me. Perhaps that's cause I started coding with phpBB and it was using snake_case for all it's PHP functions. Now when I am rather comfortable with all kinds of coding languages, I don't really mind camelCase and PascalCase anymore. Eventhough I don't prefer PascalCase. And now I've been working with multiple languages and frameworks, I always prefer to code in the same style as is common practise for the resepective language/framework. So my suggestion would be to keep using snake_case for Twig.

I agree that there should/could be some guidelines for twig functions. Especially cause there are already guidelines for php and js. The main one that comes to my mind, is that a function should contain a verb and a noun. The verb would be 'what are we doing' and the noun would be 'on what are we doing something'. I think that will be better than prefixing functions aswell. I only fail to see how capitalizing a word or words, makes the Clarity and Confidence (read 'clearer') any better. As it currently stands, working with twig, we have lower case conditionals ({% if %}, {% for %}), upper snake case variables ({{ MY_VARIABLE }}) and lower snake case functions and filters (random() and |e('js')). For clarity's sake, I would stick to those.

While these functions might be mostly visible on the front-end, you can not separate them from the back-end. As most of these functions are (or will) do or get something in the back end. For example the twig function {{ lang('') }} is linked to the php $language->lang('') function. So naming front-end functions differently as their respective back-end counterparts seems really odd. You might then satisfy the novice front-end user, but you will confuse everyone that gets a little deeper into the code.

What I disagree with, is building code for people unfamilar with the codebase. Users will have to get familar with the framework they're working with, if they want to alter it. And the fastest way to get familar with a framework, is if the framework is consistent. No matter the location. And if they're unfamilar with the fact that word() is a function, then they've got a long way to go. No matter how well the function name is formatted.

I would also like to add, that we really have to get rid of the extensions BC conversation for 4.0. It's not viable. There will be a lot of new things, some already integrated, some still in the pipelines, that will make keeping backwards compatibility for extensions close to impossible. Or atleast to say the least; less than ideal. It will just bloat the code. Just to name a few: avatars, modules, templates, events ?, possibly database, etc.. We shouldn't aim to destroy BC and if possible provide it, but I feel like it should most definitely not be a blocker for new features, functions and options.

Anyways, long story short:
Get rid of the current '1 word functions' in twig and start using self-explanatory functions, just as in the back-end: get_icon, display_avatar, create_jumpbox etc..

[hanakin]

you are still missing the point. 1st totally have to disagree with the notion that you have to get familiar with the backend...You should never force anyone to have to do this. In fact its a bad practice and goes against the concept of seperation of concerns. 2nd I agree naming has to be descriptive. I am not saying that it should not be. In fact quite the opposite the function call alone should aim to provide as much information as possible to the style developer, which is why I suggested a scope in the name. Next you are interpreting clarity to the issue incorrectly. clarity is not a per language thing clarity applies to the entire code base that comprises the file. which is the case of the front-end is a composite of html, twig/php, js, css. This means that you have to balance all of them as one which is what we currently are failing at. The idea of noun_verb is irrelevant to the clarity of the front-end. Front-end does not work in the REST world so using REST/CRUD operation names is not necessary and probably makes things more confusing. You need to look at how the html, css, js naming is defined. Rather than following a php naming scheme it should all follow a front-end or template naming scheme. You can easily map them to whatever you want in the back-end as I stated earlier its irrelevant to the front-end. As to the point of confusing the backend...Totally disagree thats what docblocks documentation, and IDEs are for...

[martti]

The fact you propose something like

Append twig_ for all the functions. This one is the most specific and descriptive and follows the same guidelines as the approach used for CSS.

Doesn't that show that you are actually trying to cater for exactly the opposite, namely mixing contexts?

The context should be already clear. In a twig file (and in the template folder), you'll have twig functions, no need to prefix them with twig_.

That being said, it would help if twig filenames had the correct extension or suffix .twig or .html.twig in phpBB. Most code editors recognize twig files. The template files are not html!

development_board_martti

[hanakin]

first the file ext will change in 4.0 in the new theme

as to the appending not sure i understand what you are saying the idea is that when using it in html it needs to be differentiated from everything else in the file as is the practice to be-able to better understand where something is coming from.

You can not remove everything from the html to completely separate concerns as you can in the backend. To solve this we append a naming convention to help easily identify this.