Add an option to only show dictionaries basename

[user202729]

Update later: conflicts resolved, however it may be a better idea to use Benoit's idea below to "smartly" display the name based on least-to-avoid-conflict.

---

Summary of changes

Add an option to only show dictionaries basename.

Closes #973.

By the way: annoyingly, the default value have to be duplicated at two places. (note needs discussion, see #1162 (comment))

Pull Request Checklist

• Changes have tests
• News fragment added in news.d. See documentation for details

[nvdaes]

Is this PR updated? Now Plover uses GitHub Actions and items of the checklist are not checked.

[user202729]

Conflicts resolved.

As I pointed out, I don't really like having the configuration default duplicate, and I might want to look at that first before writing the news snippets (which is not that hard).

GUI tests remain not possible with the current testing tools. (note configuration testing might be possible)

[user202729]

Update: turns out that it still works even if it isn't set in the __init__ (i.e., the configuration update is always called with all the keys on startup even if there's no entry for it in plover.cfg)

So currently there are 3 types of objects with this problem

1. Those which calls on_config_changed(engine.config) in __init__ (usually Qt components)
2. Those with the information duplicated, and can be used independently
3. Those without the information duplicated, can be used independently but the user must manually set the setting before it can be used

I implemented user202729@48b7776 (note I was wrong partially, the initial config_changed in some qt gui elements cannot be removed, fixed in user202729@86f2ad4 , might still have bugs, TODO investigate), which change all objects of
type 1 to not call on_config_changed on init (since it will be called (at least this is guaranteed by the engine at the moment)),
and all objects of type 2 to type 3.

What do you think? How should these objects be designed?

Note It's possible to make keep_selection work even if the dictionaries widget is not configured like this

```diff diff --git a/plover/gui_qt/dictionaries_widget.py b/plover/gui_qt/dictionaries_widget.py index a0c99f9..5713e62 100644 --- a/plover/gui_qt/dictionaries_widget.py +++ b/plover/gui_qt/dictionaries_widget.py @@ -166,7 +166,7 @@ class DictionariesWidget(QWidget, Ui_DictionariesWidget): record=False, save=False, ) if update_kwargs: - self._update_dictionaries(**update_kwargs, keep_selection=self._configured) + self._update_dictionaries(**update_kwargs)

 def _update_dictionaries(self, config_dictionaries=None, loaded_dictionaries=None,
                          reverse_order=None, record=True, save=True,

@@ -321,6 +321,7 @@ class DictionariesWidget(QWidget, Ui_DictionariesWidget):

 def _get_selection(self):
     row_list = [item.row() for item in self.table.selectedItems()]

•    if not row_list: return []
     if self._reverse_order:
         row_count = len(self._config_dictionaries)
         row_list = [row_count - row - 1 for row in row_list]
  

</details>

[nvdaes]

Hello, I've tested this locally and it works for me running from source.
Anyway, I'd add an option to show the path for each dictionary in the context menu, opening a properties dialog or something like that, since two dictionaries may have the same name.

You forked Plover, so I guess you aren't a core developer. Should we open a discussion or ping them to see their point of view?

Cheers

[user202729]

They get notified anyway. (plus the 73ish people who watch the repository on GitHub?) Just wait for a day or two.

By the way Benoit also have a fork of Plover despite having write access to the repository.

---

I'd add an option to show the path for each dictionary in the context menu, opening a properties dialog or something like that, since two dictionaries may have the same name.

That's quite complex (and also somewhat conflict with this feature).

There's already a context menu; so if the idea is extended a little there would be "Change dictionary display name..."); however I'm not very good at designing (see what happened with the save-as pull request?)

[nvdaes]

I didn't see the PR about save as.

Thanks for your work.

[user202729]

For what it's worth Benoit basically threw it away and reimplement it so... but this is getting chatty.

[benoit-pierre]

There's already a way to get short names: store your dictionaries in the config folder. Why is that not an option?

#973 mention using Dropbox, can't it be configured to sync the config folder? Is the issue that it's syncing too much?

Anyway, I specifically don't want yet another configuration option. The support for "show dictionaries in reverse order" is only there because that's how it was done in 3.x (which never made sense to me). I'm tempted to remove it.

So let's make the code smarter instead: we could find the shortest path suffix for each entry so the list is unambiguous. Maybe even get rid of the extensions.

Anyway, I'd add an option to show the path for each dictionary in the context menu, opening a properties dialog or something like that, since two dictionaries may have the same name

Each entry has a tooltip, showing the full path, are tooltips accessible in any way when using a screen reader?

[user202729]

I think the main reason why someone would not place their dictionary in Plover's configuration dictionary...

• Convenience (for example, someone downloads a dictionary into the default download folder, then adds it to Plover immediately -- rather than move it to Plover's main configuration dictionary then add it to Plover) -- perhaps this isn't a good idea.
• Dropbox sync and stuff (if it could be configured to sync more than one folders it might work, but then you have to exclude main.json and plover.cfg)
• Existing path to a git repository (moving repositories to Plover configuration folder is not very convenient)

Besides, currently Plover normalizes the path (symlinks) so placing the dictionary somewhere else and sym link it to Plover CONFIG_DIR doesn't work.

Shortest path... that's an okay idea. (comment: If there's a/b/c.json and d/b/c.rtf should the minimal distinguishable be a/b/c / d/b/c or c.json / c.rtf?)

For configuration option, it would be a little better if there was GUI tests I guess...?

[benoit-pierre]

Shortest path... that's an okay idea. (comment: If there's a/b/c.json and d/b/c.rtf should the minimal distinguishable be a/b/c / d/b/c or c.json / c.rtf?)

I'd go with the simplest option: a/b/c / d/b/c.

WIP:

--- c/plover/gui_qt/dictionaries_widget.py
+++ w/plover/gui_qt/dictionaries_widget.py
@@ -21,7 +21,7 @@
 from plover.config import DictionaryConfig
 from plover.dictionary.base import create_dictionary
 from plover.engine import ErroredDictionary
-from plover.misc import normalize_path
+from plover.misc import normalize_path, path_list_unique_suffixes
 from plover.oslayer.config import CONFIG_DIR
 from plover.registry import registry
 from plover import log
@@ -180,11 +180,12 @@ def _update_dictionaries(self, config_dictionaries=None, loaded_dictionaries=Non
         self._updating = True
         self.table.setRowCount(len(config_dictionaries))
         favorite_set = False
+        path_suffixes = path_list_unique_suffixes(d.path for d in config_dictionaries)
         for n, dictionary in enumerate(config_dictionaries):
             row = n
             if self._reverse_order:
                 row = len(config_dictionaries) - row - 1
-            item = QTableWidgetItem(dictionary.short_path)
+            item = QTableWidgetItem(os.path.splitext(path_suffixes[dictionary.path])[0])
             item.setFlags((item.flags() | Qt.ItemIsUserCheckable) & ~Qt.ItemIsEditable)
             item.setCheckState(Qt.Checked if dictionary.enabled else Qt.Unchecked)
             item.setToolTip(dictionary.path)
diff --git c/plover/misc.py w/plover/misc.py
index 3a41ee26..f6fd6f58 100644
--- c/plover/misc.py
+++ w/plover/misc.py
@@ -1,10 +1,12 @@
 # Copyright (c) 2016 Open Steno Project
 # See LICENSE.txt for details.
 
+from pathlib import Path
+import itertools
 import os
 
 from plover.oslayer.config import CONFIG_DIR
-from plover.resource import ASSET_SCHEME
+from plover.resource import ASSET_SCHEME, resource_split_asset
 
 
 def popcount_8(v):
@@ -77,3 +79,39 @@ def to_surrogate_pair(char):
         else:
             pairs.append(code_point)
     return pairs
+
+def path_list_unique_suffixes(path_list):
+    path_list = list(path_list)
+    prefixes = {}
+    def path_parts(p):
+        return tuple(part + os.path.sep for part in Path(p).parts)
+    for p in path_list:
+        if p.startswith(ASSET_SCHEME):
+            package, resource = resource_split_asset(p)
+            parts = (ASSET_SCHEME,) + (package + ':',) + path_parts(resource)
+        else:
+            parts = path_parts(normalize_path(p))
+        prefixes[p] = [1, parts]
+    # Don't allow duplicates.
+    assert len(set(map(tuple, prefixes.values()))) == len(path_list)
+    def sort_key(i):
+        p, (count, parts) = i
+        return parts[-count:]
+    has_conflicts = True
+    while has_conflicts:
+        has_conflicts = False
+        for k, conflicts in itertools.groupby(sorted(prefixes.items(),
+                                                     key=sort_key),
+                                              key=sort_key):
+            conflicts = list(conflicts)
+            if len(conflicts) == 1:
+                continue
+            has_conflicts = True
+            for p, (count, parts) in conflicts:
+                count += 1
+                assert count <= len(parts)
+                prefixes[p][0] = count
+    return {
+        p: os.path.normpath(''.join(parts[-count:]))
+        for p, (count, parts) in prefixes.items()
+    }
diff --git c/plover/resource.py w/plover/resource.py
index 715b9238..1e0d5b9f 100644
--- c/plover/resource.py
+++ w/plover/resource.py
@@ -9,7 +9,8 @@
 
 ASSET_SCHEME = 'asset:'
 
-def _asset_split(resource_name):
+def resource_split_asset(resource_name):
+    assert resource_name.startswith(ASSET_SCHEME)
     components = resource_name[len(ASSET_SCHEME):].split(':', 1)
     if len(components) != 2:
         raise ValueError('invalid asset: %s' % resource_name)
@@ -17,12 +18,12 @@ def _asset_split(resource_name):
 
 def resource_exists(resource_name):
     if resource_name.startswith(ASSET_SCHEME):
-        return pkg_resources.resource_exists(*_asset_split(resource_name))
+        return pkg_resources.resource_exists(*resource_split_asset(resource_name))
     return os.path.exists(resource_name)
 
 def resource_filename(resource_name):
     if resource_name.startswith(ASSET_SCHEME):
-        return pkg_resources.resource_filename(*_asset_split(resource_name))
+        return pkg_resources.resource_filename(*resource_split_asset(resource_name))
     return resource_name
 
 def resource_timestamp(resource_name):

[nvdaes]

There's already a way to get short names: store your dictionaries in the config folder. Why is that not an option?

For me, now it's an option. Not before, since there was not an easy way to open the configuration folder.

Each entry has a tooltip, showing the full path, are tooltips accessible in any way when using a screen reader?

Yes. The only problem is to know the meaning of the word "tooltip" for a developer who designs a gui and for the screen reader. For example, NVDA is always reading the full path of dictionaries not stored in the configuration folder and I have the screen reader option to report tooltips disabled. NVDA has a panel in the settings dialog, named Object presentation (`control+NVDA key + o), where we can choose that some object properties or object based on their roles are or not spoken, for example object description, tooltips, notifications, object shortcut, etc.

[user202729]

Weird corner case test:

print(path_list_unique_suffixes(
	['/a/b/c', '/b/c']
	))
# -> {'/a/b/c': 'a/b/c', '/b/c': '//b/c'}

...why are there two slashes?

[benoit-pierre]

Are you on Windows?

[benoit-pierre]

Never mind, os.path.normpath does not touch the initial slashes:

In [2]: os.path.normpath('//c//test')
Out[2]: '//c/test'

[benoit-pierre]

But os.path.realpath does:

>>> os.path.realpath('//test')
'/test'

o.O

[benoit-pierre]

--- i/plover/misc.py
+++ w/plover/misc.py
@@ -84,7 +84,8 @@ def path_list_unique_suffixes(path_list):
     path_list = list(path_list)
     prefixes = {}
     def path_parts(p):
-        return tuple(part + os.path.sep for part in Path(p).parts)
+        return tuple(part + ('' if part.endswith(os.path.sep) else os.path.sep)
+                     for part in Path(p).parts)
     for p in path_list:
         if p.startswith(ASSET_SCHEME):
             package, resource = resource_split_asset(p)
@@ -112,6 +113,6 @@ def sort_key(i):
                 assert count <= len(parts)
                 prefixes[p][0] = count
     return {
-        p: os.path.normpath(''.join(parts[-count:]))
+        p: ''.join(parts[-count:])
         for p, (count, parts) in prefixes.items()
     }

This will need to be tested with UNC paths on Windows.

[user202729]

Note that realpath also resolve symlinks (which might make dictionary name longer, depends on whether the user symlinks dictionaries somewhere into Plover's configuration dictionary) -- but with the change it doesn't matter.

Note: https://bugs.python.org/issue636648 (not a bug, intentional) ; https://unix.stackexchange.com/questions/12283/unix-difference-between-path-starting-with-and (double initial slashes are implementation-defined, for example //host/path on cygwin) so it's better to not generate them in the first place. (weird) (does anyone run Plover under cygwin instead of native Windows?)