-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Replace Nullable{T} with Union{Some{T}, Void} #23642
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
0b0bc91
Replace Nullable{T} with Union{Some{T}, Void}
nalimilan 605f01a
Use Union{T, Void} rather than Union{Some{T}, Void} where possible
nalimilan 5c04391
Update precompile.jl
nalimilan b7cb560
Review fixes
nalimilan 5db20e3
Add coalesce() and use it
nalimilan a2f1799
Fix start_worker() method definition
nalimilan c0053f0
Add notnothing(x) to assert that x !== nothing
nalimilan ec91f63
Fix ProductIterator regression by using custom MaybeValue wrapper
nalimilan c8b6193
Improve NEWS
nalimilan bfb40cf
Make tryparse() return Union{T, Void}
nalimilan 833c9ea
Improve Some, remove get(::Some[, x]) method in favor of coalesce (no…
nalimilan c8ba97a
LibGit2 rebase fixes
nalimilan e44a7df
Test fixes
nalimilan 1605c9d
Remove test/nullable.jl
nalimilan e0770e5
Fix remaining call to get() on Windows
nalimilan 55b03b1
Support missing in coalesce()
nalimilan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1512,8 +1512,8 @@ end | |
function prompt(msg::AbstractString; default::AbstractString="", password::Bool=false) | ||
Base.depwarn(string( | ||
"`LibGit2.prompt(msg::AbstractString; default::AbstractString=\"\", password::Bool=false)` is deprecated, use ", | ||
"`get(Base.prompt(msg, default=default, password=password), \"\")` instead."), :prompt) | ||
Base.get(Base.prompt(msg, default=default, password=password), "") | ||
"`result = Base.prompt(msg, default=default, password=password); result === nothing ? \"\" : result` instead."), :prompt) | ||
coalesce(Base.prompt(msg, default=default, password=password), "") | ||
end | ||
end | ||
|
||
|
@@ -1658,20 +1658,20 @@ import .Iterators.enumerate | |
|
||
# PR #23640 | ||
# when this deprecation is deleted, remove all calls to it, and replace all keywords of: | ||
# `payload::Union{CredentialPayload,Nullable{<:Union{AbstractCredential, CachedCredentials}}}` | ||
# `payload::Union{CredentialPayload, AbstractCredential, CachedCredentials, Void}` | ||
# with `payload::CredentialPayload` from base/libgit2/libgit2.jl | ||
@eval LibGit2 function deprecate_nullable_creds(f, sig, payload) | ||
if isa(payload, Nullable{<:Union{AbstractCredential, CachedCredentials}}) | ||
if isa(payload, Union{AbstractCredential, CachedCredentials, Void}) | ||
# Note: Be careful not to show the contents of the credentials as it could reveal a | ||
# password. | ||
if isnull(payload) | ||
msg = "LibGit2.$f($sig; payload=Nullable()) is deprecated, use " | ||
if payload === nothing | ||
msg = "LibGit2.$f($sig; payload=nothing) is deprecated, use " | ||
msg *= "LibGit2.$f($sig; payload=LibGit2.CredentialPayload()) instead." | ||
p = CredentialPayload() | ||
else | ||
cred = unsafe_get(payload) | ||
cred = payload | ||
C = typeof(cred) | ||
msg = "LibGit2.$f($sig; payload=Nullable($C(...))) is deprecated, use " | ||
msg = "LibGit2.$f($sig; payload=$C(...)) is deprecated, use " | ||
msg *= "LibGit2.$f($sig; payload=LibGit2.CredentialPayload($C(...))) instead." | ||
p = CredentialPayload(cred) | ||
end | ||
|
@@ -3244,6 +3244,11 @@ end | |
@deprecate indices(a) axes(a) | ||
@deprecate indices(a, d) axes(a, d) | ||
|
||
@deprecate_moved Nullable "Nullables" | ||
@deprecate_moved NullException "Nullables" | ||
@deprecate_moved isnull "Nullables" | ||
@deprecate_moved unsafe_get "Nullables" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
|
||
# END 0.7 deprecations | ||
|
||
# BEGIN 1.0 deprecations | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want a similar function to the
get
above? it seems to be a standard pattern, which becomes a bit verbose now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as I noted above it would make sense to have a convenience function for that (not sure what it would be exactly). But it can always be added later (contrary to API breaks).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I forgot to reread those last comments in this thread! If no good name is found before this gets merged, I think I would be in favor of using a non-exported temporary name, like
_get
or whatever, so that at least the pattern is captured (and easily replaced when the new name comes), and at least something is available for the code which will be written in the meantime.