Closed nvdaes closed 1 year ago
Hi Cyrille, this review is amazing! I didn't know that single quotes should be used before string in confspec. Let's see how alt+1, alt+2 etc. are recognized. I tried it and I think that it wasn't recognized, so finally I decided to use just alt+d for dot 1.
I didn't know that single quotes should be used before string in confspec.
No, the problem is not this. Actually, you can use either quote to define strings in Python and the strings in the confSpec's dictionaries are not an exception.
But originally, you had writen:
"string(default="")"
In Python, this defines consecutive strings, and in Python, two consecutive strings are just concatenated. The following 3 commands in the console just define the same thing:
>>> "string(default="")"
'string(default=)'
>>> "string(default=" ")"
'string(default=)'
>>> "string(default=" + ")"
'string(default=)'
The issue with configSpec is that you need to define a value defaulting to empty string as follows:
string(default="")
Instead you hade defined it as follows, which is not valid:
string(default=)
Let's see how alt+1, alt+2 etc. are recognized. I tried it and I think that it wasn't recognized, so finally I decided to use just alt+d for dot 1.
Well, let's forget this solution; actually I had not tested it. On French keyboard, you need to press alt+shift+1 to get it work, maybe because to get 1 on main (alpha-numeric) keyboard, you need to press shift+1 to actually get the 1. So keeping the 'd' shortcut is finally a better solution. Sorry for this unadapted proposal.
Don"t worry. Your review is excellent. I"m creating an isvalid method to analize if keys are repeated, for example, using the same key for different dots or null keys. If you have time and would like to make comments, they are appreciated.
Hi Cyrille, thanks for the explanation about strings too.
Thanks P for your rev:ew and approval. I"ll merge this tomorrow. Good night.Enviado desde mi iPhoneEl 7 ene 2023, a las 22:29, Cyrille Bougot @.***> escribió: @CyrilleB79 approved this pull request.
Thanks for the modifications. Seems OK for the parts I have reviewed. Note: I have not made an exhaustive review of the whole code, but rather reviewed the UX and digged into issues found.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this becauseThanks Cyrille for your review and approval. I"ll merge tis tomorrow Message ID: @.***>
Hi Cyrille, thanks for the explanation about strings too.
Hi Noelia A bit off-topic, but regarding empty strings in the configSpec, I just realize by chance that the same issue is present in ClipContentDesigner's code. No functional issue thanks to the try/except statements; but you may want to fix it to get a more logical and understandable code. Feel free to Cc me if you do so.
Thanks as always. I'll cc you when I can address it for clipContentsDesigner.
2023-01-09 23:04 GMT+01:00, Cyrille Bougot @.***>:
Hi Cyrille, thanks for the explanation about strings too.
Hi Noelia A bit off-topic, but regarding empty strings in the configSpec, I just realize by chance that the same issue is present in ClipContentDesigner's code. No functional issue thanks to the try/except statements; but you may want to fix it to get a more logical and understandable code. Feel free to Cc me if you do so.
-- Reply to this email directly or view it on GitHub: https://github.com/nvdaes/pcKbBrl/pull/10#issuecomment-1376389846 You are receiving this because you modified the open/close state.
Message ID: @.***>
Link to issue number:
Fixes issue #9
Summary of the issue:
Some people may need to configure dots and keys to be ignored when typing in braille.
Description of how this pull request fixes the issue:
Testing performed:
Known issues with pull request:
None
Change log entry: