Skip to content
This repository has been archived by the owner on Mar 12, 2020. It is now read-only.

Numeric parsing: ensure params array is correctly indexed with no missing keys #25

Merged
merged 1 commit into from
May 16, 2015
Merged

Conversation

Renegade334
Copy link
Contributor

Due to the way explode() is used in params parsing for numeric, the $parsed['params'] array is currently 1-indexed rather than 0-indexed.

In addition, if an empty param is filtered out, it leaves a "hole" in the index sequence.

ie. :server.name 001 Phergie3 :Welcome to the network becomes:

array(
  1 => 'Welcome to the network',
)

and :server.name 005 Phergie3 CHANMODES=b,k,l,nt TOPICLEN=30 :are supported by this server becomes:

array(
  1 => 'CHANMODES=b,k,l,nt',
  3 => 'TOPICLEN=30',
  4 => 'are supported by this server',
)

This changeset re-indexes the params array to ensure the array keys are always sequential integers beginning at 0. With it, the second example above becomes:

array(
  0 => 'CHANMODES=b,k,l,nt',
  1 => 'TOPICLEN=30',
  2 => 'are supported by this server',
)

Comments needed, as this is a breaking change for any plugin that uses a parameter at a specific index when parsing a numeric. This was also the case with #14, and at the time, the plugins known to be affected were UserMode and OpsWorksDeployment.

@elazar
Copy link
Member

elazar commented May 14, 2015

The 1- vs 0-index quality was partly done intentionally so that indexes would correspond to ordinal argument numbers (i.e. 1 for the first argument, 2 for the second, etc.) to make referencing them in conversation easier.

I can see how filtering empty values making the keys non-sequential would be undesirable behavior, though, and I'm fine with that aspect of this change set. I agree changing the index start bears further discussion if we're going to commit to breaking any plugins currently reliant on that behavior.

@Renegade334
Copy link
Contributor Author

array_combine(range(1, count($params)), $params)

perhaps...

@elazar
Copy link
Member

elazar commented May 14, 2015

👍

@Renegade334 Renegade334 changed the title Numeric parsing: ensure params array is 0-indexed with no missing keys Numeric parsing: ensure params array is correctly indexed with no missing keys May 14, 2015
@Renegade334
Copy link
Contributor Author

Ready for merging.

elazar added a commit that referenced this pull request May 16, 2015
Numeric parsing: ensure params array is correctly indexed with no missing keys
@elazar elazar merged commit 478ba26 into phergie:master May 16, 2015
@Renegade334 Renegade334 deleted the devel branch May 27, 2015 13:28
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