Skip to content

Conversation

@lukaszsamson
Copy link
Contributor

Fixes #11110
Handles cases not covered by 13ba96b

There are 2 more usages but those look safe. If I understand it correctly they happen only on compile time

end

# TODO: Remove me on v2.0
def reduce(%{__struct__: Range, first: first, last: last} = range, acc, fun) do
Copy link
Member

Choose a reason for hiding this comment

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

I believe we don't need this clause because it will fallback to the protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right

iex(6)> Enum.reduce(old_range, 0, fn a, b -> a + b end)
55

@josevalim
Copy link
Member

Thanks @lukaszsamson, however I believe those changes are not necessary, except for the daterange ones. Can you please keep only them and add some tests? thank you.

@lukaszsamson
Copy link
Contributor Author

@josevalim I reverted not needed changes and added test coverage

Copy link
Contributor

@v0idpwn v0idpwn left a comment

Choose a reason for hiding this comment

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

Nitpick for consistency


# TODO: Remove me on v2.0
def member?(
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
%Date.Range{first_in_iso_days: first_days, last_in_iso_days: last_days} =

Copy link
Member

Choose a reason for hiding this comment

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

We are not pattern matching on range on purpose because those are "old versions" of the range. In theory the pattern works, because we don't check the size, but I would prefer to keep it explicit it is a subset.

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL, sorry for noise :)

Comment on lines +82 to +88
def slice(
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
slice(Map.put(date_range, :step, step))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def slice(
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
slice(Map.put(date_range, :step, step))
end
def slice(
%Date.Range{first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
slice(Map.put(date_range, :step, step))
end

Comment on lines +116 to +123
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range,
acc,
fun
) do
step = if first_days <= last_days, do: 1, else: -1
reduce(Map.put(date_range, :step, step), acc, fun)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range,
acc,
fun
) do
step = if first_days <= last_days, do: 1, else: -1
reduce(Map.put(date_range, :step, step), acc, fun)
end
%Date.Range{first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range,
acc,
fun
) do
step = if first_days <= last_days, do: 1, else: -1
reduce(Map.put(date_range, :step, step), acc, fun)
end

Comment on lines +173 to +181
# TODO: Remove me on v2.0
defp size(
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
size(Map.put(date_range, :step, step))
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# TODO: Remove me on v2.0
defp size(
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
size(Map.put(date_range, :step, step))
end
# TODO: Remove me on v2.0
defp size(
%Date.Range{first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
size(Map.put(date_range, :step, step))
end

Comment on lines +200 to +207
# TODO: Remove me on v2.0
defp empty?(
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
empty?(Map.put(date_range, :step, step))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# TODO: Remove me on v2.0
defp empty?(
%{__struct__: Date.Range, first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
empty?(Map.put(date_range, :step, step))
end
# TODO: Remove me on v2.0
defp empty?(
%Date.Range{first_in_iso_days: first_days, last_in_iso_days: last_days} =
date_range
) do
step = if first_days <= last_days, do: 1, else: -1
empty?(Map.put(date_range, :step, step))
end

Comment on lines +220 to +225

# TODO: Remove me on v2.0
def inspect(%{__struct__: Date.Range, first: first, last: last} = date_range, opts) do
step = if first <= last, do: 1, else: -1
inspect(Map.put(date_range, :step, step), opts)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# TODO: Remove me on v2.0
def inspect(%{__struct__: Date.Range, first: first, last: last} = date_range, opts) do
step = if first <= last, do: 1, else: -1
inspect(Map.put(date_range, :step, step), opts)
end
# TODO: Remove me on v2.0
def inspect(%Date.Range{first: first, last: last} = date_range, opts) do
step = if first <= last, do: 1, else: -1
inspect(Map.put(date_range, :step, step), opts)
end

@josevalim josevalim merged commit cede1a4 into elixir-lang:master Jul 14, 2021
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Copy link

@Carmen675 Carmen675 left a comment

Choose a reason for hiding this comment

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

I DONT CODE I SONT KNOW HOW I JUST WANT WHO EVER HAS TAKEN OVER MY PHONE TO PLEASE STOP PUTTING MALWARE ON MY IPHONE AND SLOWLY TURNING MY PHONE INTO A ANDROID THATS GOING TO BE THEIRS. I HAVE HAD NO IDEA SOMEONE WAS DOING THIS TRACKING MY EVERY MOVE, RECORDING MY PHONE CALLS WHEN I USED TO GET THEM NOW NO CALLS OR TEXTS HARDLY ANYTHING AT ALL

@lukaszsamson lukaszsamson deleted the ls-fix-range-breaking-changes branch April 26, 2024 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Breaking changes in Range on 1.12

4 participants