Fix punctuation/symbols dialog not saving

[SaschaCowley]

Link to issue number:

Fixes #17344

Summary of the issue:

The punctuation/symbols dialog would not close when ok was pressed.

Description of user facing changes

The dialog now closes as expected.

Description of development approach

Explicitly pass in the name of the symbols file when attempting to save from the symbols dialog.

Testing strategy:

Manually tested saving with a blank config and an existing config.

Known issues with pull request:

Decision needs to be made as to whether this warrants a patch release.

Code Review Checklist:

• Documentation:
  • Change log entry
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• API is compatible with existing add-ons.
• Security precautions taken.

@coderabbitai summary

[XLTechie]

Decision needs to be made as to whether this warrants a patch release.

Given that the ability to edit symbols appears quite disabled without this patch, IMO it surely does warrant a release.

[nvdaes]

I agree with Luke. I think that a maintenance release should be done.

[XLTechie]

Shouldn't we have system tests for this kind of thing?

[SaschaCowley]

@XLTechie and @nvdaes I wholeheartedly agree that this warrants a patch release. However I can't make such a decision unilaterally.

[SaschaCowley]

@XLTechie Probably, but symbol dictionaries were implemented long before system testing, I believe.

[SaschaCowley]

@LeonarddeR would you be able to take a look at this?

[nvdaes]

@SaschaCowley , I suggest you to install the Perky Duck add-on, created by me, available from the store. This add-on adds ability to use custom symbol dictionaries to process characters in Perky Duct software, and this PR seems not to be enough. When I open a document in Perky Duck with the add-on installed, I get this message:

 self.customProcessor = SymbolProcessor(pathToDict)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^

File "userConfig\addons\perky\appModules\perky.py", line 54, in init
os.path.dirname(file), SYMBOLS_DIR, os.path.basename(symbolProcessor.userSymbols.fileName)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "", line 244, in basename
File "", line 213, in split
TypeError: expected str, bytes or os.PathLike object, not NoneType

[SaschaCowley]

@nvdaes thanks. Hopefully @LeonarddeR can shed some light as to what's going wrong, as I'm not familiar with this code, and a quick scan through hasn't been enough for me to work out what's going wrong

[LeonarddeR]

I'm very sorry for the inconvenience I caused there.
Of course I don't want to justify my oversight, but the behavior of the save function is slightly strange to be honest. I.e. C{None} to use the file name last passed to L{load} or L{save}. In other words, let's implicitly save to what we loaded, whatever that was. I think that method has its problems, since it can actually save to builtin dictionaries as well.

The problem I caused is as follows:

1. First, a SymbolDictionaryDefinition object is created for the users dictionary
2. That object has a availableLocales property that fetches all symbol files for available locales. The problem here is that the symbols file does not exist yet, so availableLocales returns an empty dictionary
3. SymbolDictionaryDefinition._initSymbols doesn't load the dictionary for the locale because it does not exist. Therefore the fileName property is never set to the loaded file.

I think the current approach in this pr is excellent.
May be for 2025.1, we should consider revisiting the save function so that the fileName parameter is no longer optional.

[nvdaes]

@LeonarddeR , if you can, would you install the Perky Duck add-on maintained by me? With this PR, seems to be an error when the symbols dictionary is loaded.
If you cannot test this, I'll see how to fix it in the add-on itself. My request to you is in case this should be fixed here.

[seanbudd]

Closing in favour of #17346