some changes to make it work like Emacs vertical file completion. #185

yangwen0228 · 2022-05-02T15:18:38Z

…completion list.

…ethod

skuroda · 2022-05-06T15:21:51Z

Hey @yangwen0228 , just as a note I do see these coming in but have been busy. I appreciate you enhancing this. I see you're iterating on this. Can you add a comment when you're ready for this to be merged (and casually reviewed). I'm not up to date on the latest sublime text APIs so if you're using any of those, I'll rely on trusting what you're doing :D

yangwen0228 · 2022-05-06T16:15:05Z

OK! I find your package is the best one in the Package Control to handle the file operation. And I enhance this package to almost the same experience in Emacs. Emacs is very handy and extensible, but the speed is not quite good when use in company computers with disk encrypt software. I use this package after enhanced for hours. It is becoming handy now. But it still needs some improvements to increase the large project files sorting performance, and increase the stability when popup frequently. Maybe I should expend some time to research the partial APIs. I use the Sublime Text4, but not test in the old versions. Just wait the weekend to improve this.

…and / in anywhere.

yangwen0228 · 2022-05-10T15:58:38Z

Hey @yangwen0228 , just as a note I do see these coming in but have been busy. I appreciate you enhancing this. I see you're iterating on this. Can you add a comment when you're ready for this to be merged (and casually reviewed). I'm not up to date on the latest sublime text APIs so if you're using any of those, I'll rely on trusting what you're doing :D

I think everything works as expected, now! Do you want to have a try? Or just merge in?

skuroda · 2022-05-22T01:48:15Z

Hey @yangwen0228 catching up on this. In some chats with the SublimeText organization, we're going to move this project into their space. I'll likely let them help manage the approvals as well (because they are far more up to date than I am with practices around plugins today). I do notice the removal of some of the settings. What's the impact to those that that either used the default false value, or had it as true?

yangwen0228 · 2022-05-22T02:01:55Z

Great! No problem!

FichteFoll

I took a not so brief look as I initially intended at this and have added a few comments with some questions as well as suggestions.

Generally, I think adding a completion list is a good idea, but this is going into the wrong direction because it introduces a UX foreign to ST in general. Using the native completions feature would feel much more natural and should work better even, though it may require some more detailed knowledge on how the corresponding API needs to be handled. I wish I had the time to work on it, however, because it sounds nice.

The PR also breaks compatibility with both ST2 and ST3 at the moment and is missing a .python-vesion file to declare the plugin host since it uses 3.6+ syntax. I would be very much fine with dropping compatibility for new code & features, but moving unsupported versions to mainenance mode needs to be mentioned in the readme. (This is a heads-up for @skuroda.)

FichteFoll · 2022-06-11T17:58:39Z

advanced_new_file/commands/command_base.py

+        if "/" in path:
+            return path
+        else:
+            return ""


I don't understand this logic here, but the return value doesn't seem to be used anyway?

FichteFoll · 2022-06-11T17:59:10Z

advanced_new_file/commands/project_file_command.py

+        print(self.settings)
+        completion_delay = self.settings.get(COMPLETION_DELAY_SETTING, 100)
+        print(completion_delay)


Debug prints

FichteFoll · 2022-06-11T18:00:05Z

advanced_new_file/completions/fuzzy_sort.py

+    else:
+        choices_ratio = {}
+        for choice in choices:
+            choices_ratio[choice] = levenshtein_ratio(query, choice)


Better use a dict comprehension now.

FichteFoll · 2022-06-11T18:09:31Z

AdvancedNewFile.sublime-settings

@@ -17,9 +17,6 @@
    // A default initial value to fill the create new file input path with.
    "default_initial": "",

-    // When renaming a file it will we pre populated with the file existing filename.
-    "autofill_path_the_existing": false,


At first I questioned why the setting was removed, but in hindsight it never made it into a release (a5f062b), the name is questionable, I'm not sure if the code even works, and the behavior is somewhat as well, though I can see the appeal. Regardless, the opposite should have been the default.

A mention of that in the commit message (and having it in a separate comment) would have been nice, though.

FichteFoll · 2022-06-11T18:21:26Z

Default (Linux).sublime-keymap

+	{"keys": ["ctrl+p"],"command": "advanced_new_file_prev",
+		"context": [{"key": "setting.anf_panel"}]
+	},
+	{"keys": ["ctrl+l"],"command": "advanced_new_file_updir",


Why L? Is that how emacs does it? Please add some comments above the binding to explain their inspiration because I (for example) wouldn't know otherwise. Probably to the readme as well.

FichteFoll · 2022-06-11T18:56:29Z

Default (Linux).sublime-keymap

+	{"keys": ["enter"],"command": "advanced_new_file_confirm",
+		"context": [{"key": "setting.anf_panel"}]
+	},


Will this prevent submitting the inserted new file name when the completion list is not visible? It would also not match the native completions UX if "auto_complete_commit_on_tab": true.

FichteFoll · 2022-06-11T18:58:44Z

advanced_new_file/commands/command_base.py

+    def clear_input_view(self):
+        self.get_active_view_settings().erase("anf_input_view")
+
+    def set_input_view_content(self, content):
+        self.get_active_view_settings().set("anf_input_view_content", content)
+
+    def get_input_view_content(self):
+        return self.get_active_view_settings().get("anf_input_view_content")
+
+    def clear_input_view_content(self):
+        self.get_active_view_settings().erase("anf_input_view_content")
+
+    def clear_input_view_project_files(self):
+        self.get_active_view_settings().erase("anf_input_view_project_files")


Imo these functions provide barely any value over their bodies. Besides, I'm unsure what the purpose of get_active_view_settings is when in reality it falls back back to the window's settings?

If you want to store data per window, you should probably do that explicitly (and all the time instead of only sometimes). However, I'm unsure if a window's settings are persisted on disk on exit, so this may actually result in leaking outdated data. An alternative would be a separate mapping that associates window-specific data with the window's id and is sufficiently cleared when the panel is closed. Usually I'd suggest storing that in the window command's instance variables, but since this plugin uses multiple commands that's not really an option.

FichteFoll · 2022-06-11T18:59:47Z

advanced_new_file/commands/command_base.py

+            try:
+                input_view.hide_popup()
+            except Exception as e:
+                print("hide_popup", e)


Debug print?

advanced_new_file/completions/fuzzy_sort.py

FichteFoll · 2022-06-11T19:06:55Z

advanced_new_file/commands/project_file_command.py

+from ..anf_util import *
+
+
+class AdvancedNewFileProjectFileCommand(AdvancedNewFileNew, sublime_plugin.WindowCommand):


What is the purpose of this command?

skuroda · 2022-06-15T04:52:52Z

I took not so brief look as I initially intended at this and have added a few comments with some questions as well as suggestions.

Generally, I think adding a completion list is a good idea, but this is going into the wrong direction because it introduces a UX foreign to ST in general. Using the native completions feature would feel much more natural and should work better even, though it may require some more detailed knowledge on how the corresponding API needs to be handled. I wish I had the time to work on it, however, because it sounds nice.

The PR also breaks compatibility with both ST2 and ST3 at the moment and is missing a .python-vesion file to declare the plugin host since it uses 3.6+ syntax. I would be very much fine with dropping compatibility for new code & features, but moving unsupported versions to mainenance mode needs to be mentioned in the readme. (This is a heads-up for @skuroda.)

Also good with putting ST2 and 3 versions of the plug in into maintenance mode. I vaguely recall the package control entry having something to pin the versions there (for ST2 at the time). Am I correct in assuming that's the same action that needs to happen still?

FichteFoll · 2022-06-15T08:54:09Z

Am I correct in assuming that's the same action that needs to happen still?

Yes, it still works the same way. The suggested course of action is to create a prefixed tag for the legacy version (e.g. st3-1.7.0 which would just be a re-tag of the current latest version) and then split the release channels in two, one for ST3 and older with "tags": "st3-" and the other already existing one for the latest versions (can remain unchanged as "tags": true.

skuroda · 2022-07-16T18:43:24Z

Tag pushed and wbond/package_control_channel#8571 to pin the version.

skuroda · 2022-07-16T19:03:17Z

I've merged in a couple readme updates in #186 that I'm guessing will conflict with some of the changes in this PR.

yangwen0228 added 3 commits May 2, 2022 23:12

some changes to make it work like Emacs vertical file completion.

c6c13fd

Use enter to choose the completion candidate, and add fuzzy sort for …

988b8a5

…completion list.

A project files command is added, and improve the variable exchange m…

9066872

…ethod

Focus the opened file view always.

c4dcae7

yangwen0228 force-pushed the master branch from 89444e9 to c4dcae7 Compare May 6, 2022 15:57

Performance improved by using functools.partial, and also support ~/ …

26a17b3

…and / in anywhere.

FichteFoll reviewed Jun 11, 2022

View reviewed changes

FichteFoll mentioned this pull request Jul 19, 2022

Feature: Show directory contents with "nix" completion on double-TAB #160

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

some changes to make it work like Emacs vertical file completion. #185

some changes to make it work like Emacs vertical file completion. #185

yangwen0228 commented May 2, 2022

skuroda commented May 6, 2022

yangwen0228 commented May 6, 2022

yangwen0228 commented May 10, 2022

skuroda commented May 22, 2022

yangwen0228 commented May 22, 2022

FichteFoll left a comment •

edited

Loading

FichteFoll Jun 11, 2022

FichteFoll Jun 11, 2022

FichteFoll Jun 11, 2022

FichteFoll Jun 11, 2022

FichteFoll Jun 11, 2022 •

edited

Loading

FichteFoll Jun 11, 2022

FichteFoll Jun 11, 2022

FichteFoll Jun 11, 2022

FichteFoll Jun 11, 2022

skuroda commented Jun 15, 2022

FichteFoll commented Jun 15, 2022

skuroda commented Jul 16, 2022 •

edited

Loading

skuroda commented Jul 16, 2022

		from ..anf_util import *


		class AdvancedNewFileProjectFileCommand(AdvancedNewFileNew, sublime_plugin.WindowCommand):

some changes to make it work like Emacs vertical file completion. #185

Are you sure you want to change the base?

some changes to make it work like Emacs vertical file completion. #185

Conversation

yangwen0228 commented May 2, 2022

skuroda commented May 6, 2022

yangwen0228 commented May 6, 2022

yangwen0228 commented May 10, 2022

skuroda commented May 22, 2022

yangwen0228 commented May 22, 2022

FichteFoll left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FichteFoll Jun 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skuroda commented Jun 15, 2022

FichteFoll commented Jun 15, 2022

skuroda commented Jul 16, 2022 • edited Loading

skuroda commented Jul 16, 2022

FichteFoll left a comment •

edited

Loading

FichteFoll Jun 11, 2022 •

edited

Loading

skuroda commented Jul 16, 2022 •

edited

Loading