dictation-toolbox / Caster

Dragonfly-Based Voice Programming and Accessibility Toolkit
Other
336 stars 122 forks source link

Issue 712 #905

Closed synkarius closed 2 years ago

synkarius commented 2 years ago

Fix module/package name conflict bug

Description

The ContentRequestGenerator can now load modules with packages of the same name.

How Has This Been Tested

Manual testing + added unit test.

Types of changes

Checklist

Maintainer/Reviewer Checklist

LexiconCode commented 2 years ago

Related issue but different test procedure. Test procedure:

  1. pip install markdown
  2. restart caster
  3. say enable markdown
  4. Command isn't recognized
  5. Repeat above steps and it works if markdown package is Uninstalled
mpourmpoulis commented 2 years ago

Thanks for taking a look at this @synkarius Unfortunately I fear your solution relies among others on the assumption that the grandparent directory of The file involved is inside the sys.path So unles The Grandparent directory contains a grammar Or is the user directory itself I think the problem should persist

Also as demonstrated by @LexiconCode Because we are Appending to the seas path Site packages take precedence

synkarius commented 2 years ago

@LexiconCode, @mpourmpoulis -- thank you for the additional examples and insights. That first solution was indeed incomplete.

There are a few changes here worth pointing out: I moved the rules, hooks, and transformers dirs in the user dir into a "caster_usercontent" dir, and added __init_\.py files to all of them. I added a migrator util as well: no one should have to adjust anything manually. Any files they have in the old structure will be moved to the new structure the next time they start Caster.

(The reason why I had to change the user dir was because of the "markdown" example. If there wasn't a unique top-level package name to distinguish the user dir packages from random third party lib package names, we'd hit this problem again at some point in the future.)

I need to update the docs for the user dir changes, and there may be a few more tests I'd like to write before we merge this, but it should be ready for testing again.

synkarius commented 2 years ago

I added that test I was thinking of and updated the docs. I think I got everything in the docs, but you might want to take a quick look at that last commit I pushed and see if there's anything I obviously missed.

LexiconCode commented 2 years ago

hmm bringme.py will have to be updated with the new paths

However migrating user the generated toml might be a bit more complicated. Be aware if there is a syntax error that it reverts to the default and wiping out user bringme settings.

synkarius commented 2 years ago

Nice catch with the bringme.py paths and toml. I think migrating the config can be done safely (by thoroughly unit testing it), but I can also create a sm_bringme.toml.bak file if you think it would put people's minds at ease.

LexiconCode commented 2 years ago

), but I can also create a sm_bringme.toml.bak file if you think it would put people's minds at ease.

Yeah that would be pretty simple to do.

synkarius commented 2 years ago

That should cover everything discussed above.

LexiconCode commented 2 years ago

Bug report from chat. Reproducible by deleting the user directory. Fixed in https://github.com/dictation-toolbox/Caster/commit/0470ec9ed58a3a64f9652c331868c30b108b9be5

Error saving toml file: [Errno 2] No such file or directory: 'C:\\Users\\Main\\AppData\\Local\\caster\\settings\\settings.toml' C:\Users\Main\AppData\Local\caster\settings\settings.toml
DPI awareness could not be set; it has been set already.
Error loading _caster from D:\Backup\Library\Documents\Caster\_caster.py
Traceback (most recent call last):
  File "D:\Backup\Library\Documents\natlink\src\natlinkpy\MacroSystem\core\natlinkmain.py", line 360, in loadFile
    imp.load_module(modName,fndFile,fndName,fndDesc)
  File "C:\Users\Main\AppData\Local\Programs\Python\Python38-32\Lib\imp.py", line 234, in load_module
    return load_source(name, filename, file)
  File "C:\Users\Main\AppData\Local\Programs\Python\Python38-32\Lib\imp.py", line 171, in load_source
    module = _load(spec)
  File "<frozen importlib._bootstrap>", line 702, in _load
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "D:\Backup\Library\Documents\Caster\_caster.py", line 17, in <module>
    settings.initialize()
  File "D:\Backup\Library\Documents\Caster\castervoice\lib\settings.py", line 505, in initialize
    migrator.update_bringme_toml_to_v1_7_0()
  File "D:\Backup\Library\Documents\Caster\castervoice\lib\migration.py", line 42, in update_bringme_toml_to_v1_7_0
    if ContentRoot.USER_DIR not in directory_dict["caster rules"]:
TypeError: 'NoneType' object is not subscriptable