What file did you edit?

What file did you edit? Started by emanuele · April 18, 2014, 08:19:27 am · Read 55908 times 0 Members and 1 Guest are viewing this topic. previous topic - next topic

What file did you edit?

April 18, 2014, 08:19:27 am

This topic aims at both addon developers and "end users".

I would like to hear from you which files you were forced to edit, and how, in order to add the feature you wanted to build.

It doesn't matter how big or small the feature is, the important is to know how you want/ed to modify the code and what the project can do to allow easier extensibility.

Remember that at some point in the future (2.0? 3.0? 4.0? I don't know), the ability of the package manager to edit files may be removed in favour of an "hook/event-only" extension system, so it is important to know what are the weak points in respect to extensibility in the current codebase in order to improve them and reach a point that it will be possible to "change" almost any part of the software without having to rely on code edits.

Thanks!!

Re: What file did you edit?

Reply #1 – April 18, 2014, 11:20:27 pm

I have four custom mods, all of which have to make code edits. Most very minor one extensive.

My basic theme mod. It has to edit index.template.php to add my dark variant and remove one of the defaults. Maybe there should be a hook for setting the variants?
An image annotation mod. It allows you to draw boxes on images and leave a note beside them. The note is also inserted into the topic as a normal post. I had to edit Display.template.php to add a link from the post back to the image that was annotated.

I got around any other mods by making my own post function for it that returns both success and error messages in json instead of using the existing js post function (name escapes me right now) as that calls too many "is_allowed" functions that redirect and thus return an unparseable response to the ajax call.
An equivalent to the SMF New in Topic (or something like that) mod that adds a new indicator to each message that is new Display.template.php. This makes it much easier to see which posts are actually new since sometimes the new post doesn't actually end up at the top. (User images that are slow to load, or new responses that are shorter than the page height.) All the information is already in the message list handed to Display.template.php so it's a shame this isn't an admin option layout option.
And finally the big one, my role playing accounts mod. This lets people post from multiple accounts with only one login. Basically several accounts are created as subs to a master account so users can instantly switch accounts or post as any of them without switching. (Yes, there are other ways this could have been done, but some users really like having the role play accounts be full fledged accounts.)

I realize this mod is highly unusual, but I think it exposes some areas where the hook system is lacking that could hit mods with more general appeal. Of course if I'm missing a way to do these things without edits I'm willing to change the code.

It edits the following files

Auth.subs.php - modifies loadExistingMember to add a variable to the db queries. (The id of the parent account.) These queries could benefit from the hooks to add variables and tables like you've done with many other queries.
Auth.controller.php - I want to stop the role play accounts from logging in directly. But the only hook in action_login2 is to totally take over the login process. There is nothing between that and calling doLogin to take any action. So no extra ban or anti-spam extensions can go here either.
Post.controller.php - Multiple edits to allow users to post as or modify the posts of their role play accounts while they are logged in as a different account. I've mentioned before how every path to posting and the themes to display buttons to do what you have permission to do handles permissions to modify, remove, etc. differently. There are no hooks in Post.controller.php
Messages.subs.php - this is related to the Post stuff. Mods to checkMessagePermissions which doesn't have any hooks. Some parts of Post.controller.php call this function. Many do not.
Subs.php - this is just to fix jsmodify I think it is from setting $_SESSION['old_url']. This should really just be fixed in the baseline. Maybe ema already did. I think he was going to add ;xml to jsmodify so that 'js' doesn't have to be added to the list of things to check from in old_url.
Post.template.php - Add a select to allow the user to select which account to post as.
Display.template.php - Add a select to the quick reply form to select which account to post as.
Profile.controller.php - Modify the link tree to show the user account a role play account belongs to. It's been a while but my impression was there weren't many hooks in the profile areas.
ManageThemes.controller.php - I only allow the real user account to make theme changes and look and layout changes. Then I put mods here to set all the role play accounts to have those same settings. Again not many hooks in this area.
ProfileInfo.template.php - Show a list of role play accounts with links to their profiles under the main user.

I like hooks. I'll happily use them where I can. But I'm not too fond of your plan to get rid of code edits. Especially for the theme. Lots of mods probably want to make a small change to the theme templates and I don't see how the hooks can realistically handle all the places someone might want to inject something into the display. Also even outside the theme you simply cut down on people's options. Yes code changes are brittle. And that's a pain. But the alternative of you simply can't do that unless you willing to use your own patch system doesn't really seem like an improvement.

And huh, apparently bbcode doesn't like list in list.

Re: What file did you edit?

Reply #2 – April 23, 2014, 07:23:18 pm

Quote from: scripple – April 18, 2014, 11:20:27 pm
My basic theme mod. It has to edit index.template.php to add my dark variant and remove one of the defaults. Maybe there should be a hook for setting the variants?

That is kind of a mixed discussion, I'm not sure where to start at the moment... lol

Quote from: scripple – April 18, 2014, 11:20:27 pm
An image annotation mod. It allows you to draw boxes on images and leave a note beside them. The note is also inserted into the topic as a normal post. I had to edit Display.template.php to add a link from the post back to the image that was annotated.

I got around any other mods by making my own post function for it that returns both success and error messages in json instead of using the existing js post function (name escapes me right now) as that calls too many "is_allowed" functions that redirect and thus return an unparseable response to the ajax call.

Yeah, isAllowedTo are a pain... >_<
That is in the list of things to do for 1.1 (and Joshua already did some).
Where did you edit Display.template.php?

Quote from: scripple – April 18, 2014, 11:20:27 pm
An equivalent to the SMF New in Topic (or something like that) mod that adds a new indicator to each message that is new Display.template.php. This makes it much easier to see which posts are actually new since sometimes the new post doesn't actually end up at the top. (User images that are slow to load, or new responses that are shorter than the page height.) All the information is already in the message list handed to Display.template.php so it's a shame this isn't an admin option layout option.

I did that on my own forum...
I'd be tempted to say it should be "standard".

If nobody disagrees I may sneak it into 1.1.

So here we are talking about template_messages, right?

Quote from: scripple – April 18, 2014, 11:20:27 pm
Auth.subs.php - modifies loadExistingMember to add a variable to the db queries. (The id of the parent account.) These queries could benefit from the hooks to add variables and tables like you've done with many other queries.

Good point.
I think around in the forum there is a discussion about that function, but probably about something slightly different.

Quote from: scripple – April 18, 2014, 11:20:27 pm
Auth.controller.php - I want to stop the role play accounts from logging in directly. But the only hook in action_login2 is to totally take over the login process. There is nothing between that and calling doLogin to take any action. So no extra ban or anti-spam extensions can go here either.

You mean integrate_validate_login?
What would you be able to do exactly? Log in the "parent" account or just "kick" them? Or something else completely?

Quote from: scripple – April 18, 2014, 11:20:27 pm
Post.controller.php - Multiple edits to allow users to post as or modify the posts of their role play accounts while they are logged in as a different account. I've mentioned before how every path to posting and the themes to display buttons to do what you have permission to do handles permissions to modify, remove, etc. differently. There are no hooks in Post.controller.php

Post.controller is a mess... really... lol

I'll stop here because I'm falling asleep.

Just a thought, there are at least two general hooks for each action in Dispatcher that look like:

Code: [Select]

'integrate_action_' . action_name . '_before'
'integrate_action_' . action_name . '_after'

so, for example post and post2 the hook called are:

Code: [Select]

'integrate_action_post_before'
'integrate_action_post_after'

Yeah, the 2 is removed.
In both cases the name of the function/method is passed as argument.
You may take advantage of some of them to "cheat" and do what you want even if a specific hook is actually missing.

Quote from: scripple – April 18, 2014, 11:20:27 pm
Messages.subs.php - this is related to the Post stuff. Mods to checkMessagePermissions which doesn't have any hooks. Some parts of Post.controller.php call this function. Many do not.
Subs.php - this is just to fix jsmodify I think it is from setting $_SESSION['old_url']. This should really just be fixed in the baseline. Maybe ema already did. I think he was going to add ;xml to jsmodify so that 'js' doesn't have to be added to the list of things to check from in old_url.
Post.template.php - Add a select to allow the user to select which account to post as.
Display.template.php - Add a select to the quick reply form to select which account to post as.
Profile.controller.php - Modify the link tree to show the user account a role play account belongs to. It's been a while but my impression was there weren't many hooks in the profile areas.
ManageThemes.controller.php - I only allow the real user account to make theme changes and look and layout changes. Then I put mods here to set all the role play accounts to have those same settings. Again not many hooks in this area.
ProfileInfo.template.php - Show a list of role play accounts with links to their profiles under the main user.

I like hooks. I'll happily use them where I can. But I'm not too fond of your plan to get rid of code edits. Especially for the theme. Lots of mods probably want to make a small change to the theme templates and I don't see how the hooks can realistically handle all the places someone might want to inject something into the display. Also even outside the theme you simply cut down on people's options. Yes code changes are brittle. And that's a pain. But the alternative of you simply can't do that unless you willing to use your own patch system doesn't really seem like an improvement.

And huh, apparently bbcode doesn't like list in list.

one
two
- child one
- child two
three

Looks fine here.
Are you using the WYSIWYG editor?

Re: What file did you edit?

Reply #3 – April 24, 2014, 03:05:43 pm

Scripple, I think the best option would be to relegate code edits to a separate software. So the plugin system would handle all of the plugins, but the package manager would be separate. Composer would be the route I'd like to see.

Re: What file did you edit?

Reply #4 – April 26, 2014, 01:29:03 pm

Quote from: emanuele – April 23, 2014, 07:23:18 pmThat is kind of a mixed discussion, I'm not sure where to start at the moment... lol

How about starting with where you're going to add the hook?

QuoteWhere did you edit Display.template.php?

On the line with the Reply #x so that for image notes it reads "[image] Reply #x". Clicking on [image] takes you to the image with the annotation.

QuoteI did that on my own forum...
I'd be tempted to say it should be "standard".
If nobody disagrees I may sneak it into 1.1.
So here we are talking about template_messages, right?

I think it should be. It's not like it destroys the layout or anything. And yes in template_messages.

Quote from: scripple – April 18, 2014, 11:20:27 pm
Quote from: scripple – April 18, 2014, 11:20:27 pm
Auth.controller.php - I want to stop the role play accounts from logging in directly. But the only hook in action_login2 is to totally take over the login process. There is nothing between that and calling doLogin to take any action. So no extra ban or anti-spam extensions can go here either.
You mean integrate_validate_login?
What would you be able to do exactly? Log in the "parent" account or just "kick" them? Or something else completely?

Yes integrate_validate_login. And yes I just kick them with an error message saying to login as the main account. That way stale passwords on child accounts don't matter.

QuotePost.controller is a mess... really... lol

Yes it is.

QuoteJust a thought, there are at least two general hooks for each action in Dispatcher that look like:

Code: [Select]
'integrate_action_' . action_name . '_before'
'integrate_action_' . action_name . '_after'

I've used that but it's not much help in post and post2 unless I want to do all the work they're doing to decide if permissions are ok or not. If that can be done easily than it would better for me to just submit a PR with a simpler post and post2.

Quote
QuoteAnd huh, apparently bbcode doesn't like list in list.

Looks fine here.
Are you using the WYSIWYG editor?

No I was using the source mode editor. Maybe there's an extra [/list] in my post or something that I missed.

Re: What file did you edit?

Reply #5 – April 26, 2014, 01:30:39 pm

Quote from: groundup – April 24, 2014, 03:05:43 pmScripple, I think the best option would be to relegate code edits to a separate software. So the plugin system would handle all of the plugins, but the package manager would be separate. Composer would be the route I'd like to see.

In that case mods would have to be installed in two parts. I think that could get messy. Oh this part is only hooks and installed fine but it relied on that other part which failed due to code edits that didn't match. Now things are really hosed. Could get ugly.

Re: What file did you edit?

Reply #6 – April 26, 2014, 03:13:23 pm

Quote from: scripple – April 26, 2014, 01:29:03 pm
QuoteWhere did you edit Display.template.php?
On the line with the Reply #x so that for image notes it reads "[image] Reply #x". Clicking on [image] takes you to the image with the annotation.

Ohh, that one, yes, it's exactly where I put it as well. lol
You could actually use a trick (it could be considered ugly, but if you want to go the "hard way" about edits is a possibility): hooking integrate_prepare_display_context, you could use something like that:

Code: [Select]

function is_new_post(&$output, &$message)
{
	global $txt, $settings;
	static $txt_reply_number = null;

	// Better remember it for future reference
	if ($txt_reply_number === null)
		$txt_reply_number = $txt['reply_number'];

	// First message of the topic is a bit special
	if (empty($output['counter']))
	{
		if ($output['new'])
			$output['html_time'] = '<span class="new_posts">' . $txt['new'] . '</span>&nbsp;' . $output['html_time'];
	}
	// Any other message follow the same pattern
	else
	{
		if ($output['new'])
			$txt['reply_number'] = '<span class="new_posts">' . $txt['new'] . '</span>&nbsp;' . $txt_reply_number;
		// Better reset because it may have been changed before
		else
			$txt['reply_number'] = $txt_reply_number;
	}
}

Re: What file did you edit?

Reply #7 – April 26, 2014, 04:01:35 pm

Yeah that is pretty ugly but not as bad as what I did to the profile button to make it so that the user's avatar shows instead of the head and shoulder icon on narrow screens. I'll have to look into it when I get some time and energy.

Re: What file did you edit?

Reply #8 – April 28, 2014, 12:21:01 pm

Quote from: scripple – April 26, 2014, 01:30:39 pm
Quote from: groundup – April 24, 2014, 03:05:43 pmScripple, I think the best option would be to relegate code edits to a separate software. So the plugin system would handle all of the plugins, but the package manager would be separate. Composer would be the route I'd like to see.
In that case mods would have to be installed in two parts. I think that could get messy. Oh this part is only hooks and installed fine but it relied on that other part which failed due to code edits that didn't match. Now things are really hosed. Could get ugly.

No, Elkarte would interface with that other software or maybe the other way around.

Re: What file did you edit?

Reply #9 – June 27, 2014, 03:03:47 am

Quote from: scripple – April 26, 2014, 01:29:03 pm
Quote from: emanuele – April 23, 2014, 07:23:18 pmThat is kind of a mixed discussion, I'm not sure where to start at the moment... lol
How about starting with where you're going to add the hook?

I don't know if you noticed, but I added the hook there recently. Though it's not that useful as it may be: the problem is that with a theme you don't have any good way to unambiguously identify a theme.
From an addon point or view, you can't rely on the id (except for 0), you can barely rely on the theme name, but is a bit... odd.
This is a point that may require some work in the core: there should be a unique identifier similar to the package id of the mods stored somewhere in the database. And it's better if I stop it here, because I have some odd stuff going around in my mind related to themes...

Re: What file did you edit?

Reply #10 – August 06, 2014, 05:26:35 am

Quote from: emanuele – April 18, 2014, 08:19:27 amRemember that at some point in the future (2.0? 3.0? 4.0? I don't know), the ability of the package manager to edit files may be removed in favour of an "hook/event-only" extension system, so it is important to know what are the weak points in respect to extensibility in the current codebase in order to improve them and reach a point that it will be possible to "change" almost any part of the software without having to rely on code edits.

May be removed or has already been removed. I can't seem to create any working mod for RC2 so far.

Re: What file did you edit?

Reply #11 – August 06, 2014, 05:46:47 am

That's a bit of a general statement, there are some addons that do file edit here in this very board.
Maybe some more details could help understanding your problem.

Re: What file did you edit?

Reply #12 – August 06, 2014, 10:14:27 pm

Really?

So the package manager still working for old fashioned mod in addition to new one? So if I want to remove the header part, can I still do it via package manager?

Let say my mod is to remove header by replacing this part:

Code: [Select]

	echo '
		</div>
		<div id="header" class="wrapper', !empty($settings['header_layout']) ? ($settings['header_layout'] == 1 ? ' centerheader' : ' rightheader') : '', '"', empty($context['minmax_preferences']['upshrink']) ? '' : ' style="display: none;" aria-hidden="true"', '>
			<h1 id="forumtitle">
				<a href="', $scripturl, '">', $context['forum_name'], '</a>
			</h1>';

	echo '
			<div id="logobox">
				<img id="logo" src="', $context['header_logo_url_html_safe'], '" alt="', $context['forum_name_html_safe'], '" title="', $context['forum_name_html_safe'], '" />', empty($settings['site_slogan']) ? '' : '
				<div id="siteslogan">' . $settings['site_slogan'] . '</div>', '
			</div>';

	// Show the menu here, according to the menu sub template.
	echo '
		</div>';

To just this:

Code: [Select]

	// Show the menu here, according to the menu sub template.
	echo '
		</div>';

I created a normal old fashioned mod package, but it doesn't work so far.

Re: What file did you edit?

Reply #13 – August 06, 2014, 10:28:40 pm

To simplify things, I attached one of my trial mod that I cannot get installed by package manager. May be you can check what is wrong with it and explain since I have been trying but can't really understand what is wrong with it.

Re: What file did you edit?

Reply #14 – August 07, 2014, 05:43:05 am

The package manager emulation doesn't work "backwards".
You are specifying in your "install" (and uninstall) commands the range "1.0 - 1.0.99", but the current version is "1.0 RC2", that is lower than 1.0, so it is not installed.

I'm not sure this is a bug or not, I don't remember the emulation "specifications"^[1], @Spuds what do you think?

¹A test for that would be very useful so that we can remember how it is supposed to work (and maybe why

)↵