Skip to content

Fix: save all PGN header fields before exporting#2864

Open
MaartenD wants to merge 5 commits intolichess-org:mainfrom
MaartenD:fix/pgn-export-BlackElo
Open

Fix: save all PGN header fields before exporting#2864
MaartenD wants to merge 5 commits intolichess-org:mainfrom
MaartenD:fix/pgn-export-BlackElo

Conversation

@MaartenD
Copy link
Copy Markdown
Contributor

When pressing the share button while still in the last PGN header field (BlackElo),
the value was not saved. Fixed by reading all controllers before exporting.

Fixes #2850

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file should not be part of this PR. Especially since it does nothing here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, i will take a look and try to adjust the commit. I’m a newbie.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I hope everything is alright now, if not let me know.

@MaartenD MaartenD force-pushed the fix/pgn-export-BlackElo branch from c26d3f4 to 5c87546 Compare March 30, 2026 14:23
for (final entry in pgnHeaders.entries) {
ref
.read(ctrlProvider.notifier)
.updatePgnHeader(entry.key, _controllers[entry.key]!.text);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suppose the _controllers[entry.key] cannot be null with the current code, but we should still guard it for the future. So let's add a check to avoid the null error.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MaartenD we're still missing the null check here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@veloce when you scroll down in this list there you would see the null check. I did 2 new comits yesterday evening. I’m sorry if this is not the correct way, it’s a whole new world for me (just an old ABAP programmer ;-) ).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You added a check line 63.

There's nothing to scroll, I'm just seeing the code. Here, line 171 I see _controller[entry.key]!.text, there is still the non-null assertion operator without null check.

.gitignore Outdated

# VS Code
.vscode/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At some point we may want to include some dev tools prefs for the project so there's no reason to add this for now.

If you are using dev tools options on your own you should use .git/info/exclude to ignore it.

if (controller != null) {
ref.read(ctrlProvider.notifier).updatePgnHeader(entry.key, controller.text);
}
//ref.read(ctrlProvider.notifier).updatePgnHeader(entry.key, _controllers[entry.key]!.text);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should remove the comment

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.

Black elo missing from Sharing & export PGN from analysis board

3 participants