search

elm-community / list-extra

Convenience functions for working with List.
http://package.elm-lang.org/packages/elm-community/list-extra/latest
MIT License
135 stars 59 forks source link

isInfixOf always return True #28

Closed Mouvedia closed 7 years ago

Mouvedia commented 7 years ago

https://github.com/elm-community/list-extra/blob/master/src/List/Extra.elm#L1110

  1. tails will always generate []
  2. isPrefixOf whatever [] always return True (currently)
  3. hence isInfixOf will always return True

If I am wrong, could someone please give me an example that would return False?

mgold commented 7 years ago

isPrefixOf was fixed in #22 but not yet released. This is likely fixed by that?

Mouvedia commented 7 years ago

@mgold what should I change in my elm-package.json to use trunk instead?

mgold commented 7 years ago

elm-package.json does not support such an option.

It's the province of maintainer @abadi199 to release a new version.

What's on master is currently a breaking/major change because the argument order to andMap was changed. Hypothetically once could re-jigger git history to release just the isPrefixOf patch.

Mouvedia commented 7 years ago

@mgold so List.extra is a work in progress?

Even an explicit bold warning in the README wouldn't be enough to properly inform the users. Someone should add a suffix*—like -beta.0—to the version.

* pre-release identifier

jvoigtlaender commented 7 years ago

@Mouvedia, this package here isn't any more work in progress than any other package on http://package.elm-lang.org. Yes, the currently released version of it contains a bug. The same is probably true for other packages on the package site. There us no reason to single out this package as beta or anything. There is a process for reporting and fixing bugs, which is going on here.

But yes, the maintainer of the package should make sure that fixes are actually released in a timely manner.

jvoigtlaender commented 7 years ago

@abadi199, are you actually still able and willing to fulfill the role of maintainer as outlined in https://github.com/elm-community/Manifesto/blob/master/README.md? If not, it would be better to mark this package as unmaintained in https://github.com/elm-community/Manifesto/blob/master/maintainers.md, to facilitate finding a new maintainer.

abadi199 commented 7 years ago

Unfortunately, I don't think I will be able to allocate time to maintain this package in a timely manner anymore. I've created a PR in https://github.com/elm-community/Manifesto/pull/53 to removing myself from the maintainer of the package.

mgold commented 7 years ago

@Mouvedia Please verify the issue has been fixed with version 5.0.0.

Mouvedia commented 7 years ago

Just realized that elm-package doesn't have an upgrade command. Having to manually do it is kinda archaic.

@mgold it seems to be working fine. I gotta say that since I couldn't test it before I thought that it wouldn't match the startsWith and endsWith cases. I am pleasantly surprised. I wasn't really familiar with the term infix. Thanks!

Mouvedia commented 7 years ago

@mgold you should probably click on "Draft New Release" because 5.0.0 is currently not the latest release.

mgold commented 7 years ago

Most people in the Elm community don't bother with those release notes, but I went ahead and added it.