Skip to content
This repository was archived by the owner on Nov 28, 2017. It is now read-only.

Update jefreg.php#1

Open
mgilkes wants to merge 3 commits intojoomla-extensions:masterfrom
mgilkes:patch-1
Open

Update jefreg.php#1
mgilkes wants to merge 3 commits intojoomla-extensions:masterfrom
mgilkes:patch-1

Conversation

@mgilkes
Copy link

@mgilkes mgilkes commented Jan 18, 2014

  • fixed the regex to clean the commercial file. It was not cleaning the spaces properly.
  • fixed the regex that detects the commcerial file. Removed the colon in the expression. The expression is supposed to detect *[appid]=>[link], not *:[appid]=>[link]

- fixed the regex to clean the commercial file. It was not cleaning the spaces properly. 
- fixed the regex that detects the commcerial file. Removed the colon in the expression. The expression is supposed to detect *[appid]=>[link], not *:[appid]=>[link]
You must set the field as required. If the field is not required when $jefreg['installat'] is empty, the rule test for the field will return as true (i.e. valid).
Removed the extra "en-GB.". It only needs one. So, the file should be "en-GB.plg_system_jefreg.ini", not "en-GB.en-GB.plg_system_jefreg.ini"
@emavro
Copy link

emavro commented Jan 21, 2014

Hi, Mike!

Sorry I couldn't get back to you earlier on this. Too much work to do during this period I'm afraid.

Point one noted and will be tested.

However, the colon (:) is there to support other plugins that work with various cart systems. For example, you can compare with the jef-cbsubs branch. The regular expression there searches for

'cbsubs:'.$sessionvals['installapp']

in this plugin's parameters. In this way, you can control your redirections through the parameters of one plugin only, instead of switching from plugin to plugin to fix things.

@mgilkes
Copy link
Author

mgilkes commented Jan 21, 2014

Hi,

Thanks for replying.

Ok… I do understand why you had originally placed the colon there. I have hikashop on my site and I plan to use it with hikashop. From looking at the hikashop fork, I was able to understand that to integrate it with hikashop, you would have *hikashop:[appid]=>[link]. However, since the jefreg plugin does not handle commercial plugins, there is no need to place the colon there. The regular expression that I propose is the following:

/^.'.$appid.'=>(.+)/

I remove the colon, because .* includes the colon. Since we are not processing any particular cart systems in jefreg, we don't need to test for whether it is specified or not. So, it will ignore examples like, 

*24234=>http://example.com?com_whatever
*hikashop:23444=>http://example.com?com_whatever

So, that is why I removed the colon.

Oh, the second reason why I removed the colon is that your documentation stated that for commercial apps, they have the format: 

*777777777=>http://mywebsite.com/path/to/my/commerce/script.php 

And clearly, this never even hinted at putting a descriptive text and colon before the appid.

Hope this helps clarify why I still think the colon should be removed. Let me know how you think in regards to my response.

Michael

On Tuesday, January 21, 2014 2:49 PM, emavro notifications@github.com wrote:

Hi, Mike!
Sorry I couldn't get back to you earlier on this. Too much work to do during this period I'm afraid.
Point one noted and will be tested.
However, the colon (:) is there to support other plugins that work with various cart systems. For example, you can compare with the jef-cbsubs branch. The regular expression there searches for
'cbsubs:'.$sessionvals['installapp']
in this plugin's parameters. In this way, you can control your redirections through the parameters of one plugin only, instead of switching from plugin to plugin to fix things.

Reply to this email directly or view it on GitHub.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants