Skip to content

Conversation

vishen
Copy link
Contributor

@vishen vishen commented Nov 14, 2018

Previously, the section getKey would return keys for the first section
found. If there is more than one section with the same name, the first
section was always used when searching for keys in those sections. This
is because it loops through all the file lines looking for a section
with the same name, and the first one declared was always foudn first.

Use the section position, rather than name, to determine the section we
are looking for keys for.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 80.899% when pulling 0e4d8f2 on vishen:getkey_sections_with_same_name into 0accc53 on knq:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 80.899% when pulling 0e4d8f2 on vishen:getkey_sections_with_same_name into 0accc53 on knq:master.

@coveralls
Copy link

coveralls commented Nov 14, 2018

Coverage Status

Coverage remained the same at 80.899% when pulling e8b73a8 on vishen:getkey_sections_with_same_name into 0accc53 on knq:master.

@mvdan
Copy link

mvdan commented Nov 14, 2018

@vishen note that you seem to have broken some tests, though.

@vishen
Copy link
Contributor Author

vishen commented Nov 14, 2018

Opps, let me fix them!

Previously, the section getKey would return keys for the first section
found. If there is more than one section with the same name, the first
section was always used when searching for keys in those sections. This
is because it loops through all the file lines looking for a section
with the same name, and the first one declared was always foudn first.

Use the section position, rather than name, to determine the section we
are looking for keys for.
@vishen vishen force-pushed the getkey_sections_with_same_name branch from 0e4d8f2 to e8b73a8 Compare November 14, 2018 10:59
@vishen
Copy link
Contributor Author

vishen commented Nov 14, 2018

Fixed.

@kenshaw kenshaw merged commit e8b73a8 into kenshaw:master Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants