Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add shift and unshift method #16

Merged
merged 5 commits into from
Nov 15, 2019
Merged

add shift and unshift method #16

merged 5 commits into from
Nov 15, 2019

Conversation

tlayh
Copy link
Contributor

@tlayh tlayh commented Nov 5, 2019

  • adding a shift and unshift function to pugtemplate
  • added additional tests to existing functions

@tlayh tlayh requested a review from bastianccm November 5, 2019 09:18
@tlayh
Copy link
Contributor Author

tlayh commented Nov 5, 2019

@bastianccm The tests are only comparing the items parts of the array. Currently, the methods, also for pop and push, are not manipulating the interface part of the array. Is this fine like it is?

@bastianccm
Copy link
Contributor

Hi @tlayh what do you mean by "Currently, the methods, also for pop and push, are not manipulating the interface part of the array. Is this fine like it is?"? Can you give an example?

@tlayh
Copy link
Contributor Author

tlayh commented Nov 12, 2019

Hi @tlayh what do you mean by "Currently, the methods, also for pop and push, are not manipulating the interface part of the array. Is this fine like it is?"? Can you give an example?

Taking the unit test for unshift as an example, if I compare the the array instead of comparing the items of the array, the unit test fails with the following:

expected: &pugjs.Array{items:[]pugjs.Object{"first", "something", "test"}, o:[]string{"first", "something", "test"}}
actual : &pugjs.Array{items:[]pugjs.Object{"first", "something", "test"}, o:[]string{"something", "test"}}

The item part is correct, the o: part fails.

Does this help as an example

@bastianccm
Copy link
Contributor

bastianccm commented Nov 14, 2019

That is fine. o points to the original data the Object is created with, which usually can not be mutated.

Can you resolve the conflicts? Then we can merge it.

@tlayh
Copy link
Contributor Author

tlayh commented Nov 15, 2019

That is fine. o points to the original data the Object is created with, which usually can not be mutated.

Can you resolve the conflicts? Then we can merge it.

Resolved.

@tlayh tlayh requested a review from brnhffmnn November 15, 2019 09:40
pugjs/types.go Outdated
func (a *Array) Shift() Object {
if len(a.items) > 0 {
first := a.items[0]
a.items = a.items[1:len(a.items)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could even be shorter, couldn't it:

Suggested change
a.items = a.items[1:len(a.items)]
a.items = a.items[1:]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it. Unit tests are telling me that it is still working

@brnhffmnn brnhffmnn merged commit e58030f into master Nov 15, 2019
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.

3 participants