Skip to content

Comments

Fix deprecations#458

Merged
PetarKirov merged 4 commits intodlang-community:masterfrom
wilzbach:fix-deprecations
Jun 15, 2017
Merged

Fix deprecations#458
PetarKirov merged 4 commits intodlang-community:masterfrom
wilzbach:fix-deprecations

Conversation

@wilzbach
Copy link
Member

They were getting annoying ;-)

{
case tok!"doubleLiteral":
if (!token.text.removechars("Ll").to!double)
if (!token.text.asLowerCase.filter!(c => c != 'l').to!double)
Copy link
Member

Choose a reason for hiding this comment

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

I think the following may be shorter/faster:

auto filterChars(string chars, S)(S str)
{
	import std.algorithm.comparison : among;
    import std.algorithm.iteration : filter;
    import std.meta : aliasSeqOf;
	return str.filter!(c => !c.among(aliasSeqOf!chars));
}

if (!token.text.filterChars!"Ll".to!double)
    // ...

Copy link
Member

@PetarKirov PetarKirov left a comment

Choose a reason for hiding this comment

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

Aprart from my suggestion, LGTM.

@wilzbach
Copy link
Member Author

I think the following may be shorter/faster:

Yes, that's very elegant!

@PetarKirov
Copy link
Member

(Don't mind my stupid ws commits, I'll squash them out of existance as soon as the tests pass ;) )

@wilzbach
Copy link
Member Author

(Don't mind my stupid ws commits, I'll squash them out of existance as soon as the tests pass ;) )

No worries, I also made the same mistake here. Setting up a .editorconfig plugin in your favorite IDE can really help ;-)
Btw don't mind the test failure on dmd-nightly for now - it's a known regression in EMSI containers.

@PetarKirov
Copy link
Member

PetarKirov commented Jun 15, 2017

No worries, I also made the same mistake here. Setting up a .editorconfig plugin in your favorite IDE can really help ;-)

Ah yes, I have done so ages ago, but sometimes I'm too lazy and I edit PRs with the GitHub online editor.

Btw don't mind the test failure on dmd-nightly for now - it's a known regression in EMSI containers.

I noticed, but I can't make sense of the error - I don't think it should be there. Anyway, I think we should also bring emsi_containers to the org, so we can more easily manage such issues.

@wilzbach
Copy link
Member Author

I noticed, but I can't make sense of the error - I don't think it should be there.

It was introduced here: dlang/dmd#6816

Anyway, I think we should also bring emsi_containers to the org, so we can more easily manage such issues.

That's a great idea -> https://github.com/economicmodeling/containers/issues/81

@PetarKirov PetarKirov merged commit cf3d702 into dlang-community:master Jun 15, 2017
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.

2 participants