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

yaml bool type inconsistent boolean instance evaluation #5857

Open
timreichen opened this issue Aug 28, 2024 · 2 comments
Open

yaml bool type inconsistent boolean instance evaluation #5857

timreichen opened this issue Aug 28, 2024 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers PR welcome A pull request for this issue would be welcome yaml

Comments

@timreichen
Copy link
Contributor

Describe the bug
In yaml/_types/bool.ts, booleans are evaluated as

typeof value === "boolean" || value instanceof Boolean

This means Boolean instances are handled as booleans in yaml but the representation is evaluated as

represent: {
    lowercase(object: boolean): string {
      return object ? "true" : "false";
    },
    uppercase(object: boolean): string {
      return object ? "TRUE" : "FALSE";
    },
    camelcase(object: boolean): string {
      return object ? "True" : "False";
    },
}

This will always stringify Boolean instances to "true", which is probably not the intended behavior.

Steps to Reproduce

import { stringify } from "@std/yaml";
import { assertEquals } from "@std/assert";
assertEquals(stringify(new Boolean(false)), "true\n");

Expected behavior
new Boolean(false) should evaluate to "false" OR Boolean instances should not be special treated and value instanceof Boolean removed so they are handled as objects.

I am leaning torwards the removal. WDYT?

@timreichen timreichen added bug Something isn't working needs triage labels Aug 28, 2024
@iuioiua
Copy link
Contributor

iuioiua commented Aug 29, 2024

Yeah, it seems the predicate should just be value === true || value === false. I don't see why Boolean instances should be treated in any special way. I'm not aware of any such treatment elsewhere in the codebase.

@iuioiua iuioiua added good first issue Good for newcomers yaml PR welcome A pull request for this issue would be welcome and removed needs triage labels Aug 29, 2024
@timreichen
Copy link
Contributor Author

I'll open a PR. Would that be breaking or fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers PR welcome A pull request for this issue would be welcome yaml
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants