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

non-strict FEN validation parameter #414

Closed
wants to merge 0 commits into from
Closed

Conversation

snk-js
Copy link

@snk-js snk-js commented Jun 4, 2023

this closes #413

image

@jhlywa
Copy link
Owner

jhlywa commented Jun 4, 2023

Hi and thanks @snk-js. This change is a bit too drastic and differs from the current chess.js conventions. Some methods currently use the optional {strict: boolean()} to allow illegal or non-standard behavior. My thinking is that we would add this optional parameter to the .load() method. Unlike the other uses of strict, the default value for .load should be {strict: true}.

I don't want to extend this to the constructor at this stage, as it may imply enabling/disabling the strict/permissive parser across the object.

I'm really cautious about introducing large changes at this stage in the 1.0.0 release cycle, but I'm willing to accept a PR for the relatively minor modification to load.

Thanks again for the contribution.

@snk-js
Copy link
Author

snk-js commented Jun 4, 2023

Hi and thanks @snk-js. This change is a bit too drastic and differs from the current chess.js conventions. Some methods currently use the optional {strict: boolean()} to allow illegal or non-standard behavior. My thinking is that we would add this optional parameter to the .load() method. Unlike the other uses of strict, the default value for .load should be {strict: true}.

I don't want to extend this to the constructor at this stage, as it may imply enabling/disabling the strict/permissive parser across the object.

I'm really cautious about introducing large changes at this stage in the 1.0.0 release cycle, but I'm willing to accept a PR for the relatively minor modification to load.

Thanks again for the contribution.

I changed as follows, I had already do that way, and it was already optionally too. I do not think this is a drastic. Because we are dealing bypassing validations per se, it doesn't changed much from the original but I hope it's acceptable, many of the changes in this PR was formatting code. Let me know if this attends more briefly.

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.

can't setup initial fen without king
2 participants