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

fix issue when unit starts with dot #403

Merged

Conversation

eimerej
Copy link

@eimerej eimerej commented Sep 17, 2020

Fix for the bug #402

@dcslagel dcslagel mentioned this pull request Dec 24, 2020
@dcslagel
Copy link
Collaborator

dcslagel commented Dec 24, 2020

Draft branch at #413, demonstrates test cases for additional [name][.][unit] parsing scenarios.

The tests make assumptions about the desired behavior that should be checked. Three of the tests currently fail. This could be because the test is incorrect or because additional code changes are needed to fix the test scenario.

DC

@dcslagel
Copy link
Collaborator

dcslagel commented Mar 3, 2021

Hi @eimerej , @kinverarity1 ,

After spending a fair amount of time looking that this, @eimerej 's solution in pull-request #403 is the right one for now. It takes care of the cases .1 and .2 that I listed in issue #402 and the cases .3 and .4 are highly unlikely and can be solved if there is ever a real-world case reported. Case .5, @kinverarity1 , mentioned it is okay to let it resolve with the current logic in Laisio.

So all that said, I encourage merging pull request #403. Thank you for allowing me to dig in to this problem for an extended period of time.

DC

@kinverarity1 kinverarity1 merged commit f714cc6 into kinverarity1:master Mar 17, 2021
@kinverarity1
Copy link
Owner

Thank you @eimerej and @dcslagel - really sorry it has taken us so long here.

kinverarity1 added a commit that referenced this pull request Apr 14, 2021
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.

None yet

3 participants