Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev-mattst branch; ST2 support fixed and new folder list scrolling feature #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mattst
Copy link
Collaborator

@mattst mattst commented Dec 12, 2015

Hi there,

There is a minor change and a major change.

  • Minor: Fixed broken ST v2 support. It was broken by the switch to the more modular design and fixed by doing a version specific import of the paths and matching modules.
  • Major: New folder list scrolling feature.

The folder list scrolling is similar to the history scrolling feature. A folder list is created from the following possible locations: the user's home folder, the folder of the current file, the project folders, and additional user set folders which can be added in the settings file. The order the folders appear in can be controlled by the user in the i_opener.sublime-settings with the folder_sequence setting, and the user's additional folders can be set with the folders setting.

// `i_opener.sublime-settings`:

// User control of what order to add the folders in.
"folder_sequence": ["home", "file", "project", "user"],

// User set folders to add.
"folders":
[
     "~/Path/To/Folder/In/Home/",
     "/Full/Path/To/Folder/"
]

The i_opener.sublime-settings file has detailed instructions.

Moving up and down the folder list is almost identical to the history feature except that a modifier key is used. ctrl+up/down for Linux/Windows, super+up/down for OSX.

In addition users can change to different folders and folder lists using key bindings.

  • ctrl+h - display the home folder.
  • ctrl+f - display the folder of current file.
  • ctrl+p - load project folders, display the first one, and allow scrolling through them.
  • ctrl+u - load the user defined folders from the settings file, display the first one, and allow scrolling through them.
  • ctrl+a - load the folders list created using folder_sequence setting, display the first one, and allow scrolling through them. This is the initial folder list when the plugin starts.
  • OSX uses super in place of ctrl.

This feature has made both the get_current_directory() function and the use_project_dir setting redundant; both have been removed.

Tested and working with ST v3 and v2 on both Linux and Windows XP. Completely untested on OSX, you wouldn't happen to be an OSX user would you?

A few other minor changes were made; e.g. a few typos, comments, a variable or two renamed, and the check for ST v2 or v3 in iOpenerCommand is now set to the positive rather than the negative, in keeping with good practice.

I hope this meets with your approval.

from .matching import complete_path, COMPLETION_TYPE, get_matches
from .paths import get_current_directory, directory_listing_with_slahes
# Version specific import. Not sure if this is required due to differences in
# the way ST v2 and v3 load packages or due to the different Python versions.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, worth getting to the bottom of this probably

@rosshemsley
Copy link
Owner

Perhaps update Readme?

print("iOpener plugin is only for Sublime Text v2 and v3.")
else:

if (is_sublime_text_2() or is_sublime_text_3()):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for braces

@rosshemsley
Copy link
Owner

Looking good, will finish reviewing later!

@mattst
Copy link
Collaborator Author

mattst commented Dec 17, 2015

Hi,

Concerning your query about the import:

if python_version_major == 3:
    from .matching import ...
    from .paths import ...
elif python_version_major == 2:
    from matching import ...
    from paths import ...

It is due to the new syntax for explicit relative imports introduced in Python v3.

See the following StackOverflow answer: python: how to import the class within the same directory or sub directory

However it only seems to be required in packages, as explained at length on another StackOverflow page, here: Relative imports in Python 3

The code is a bit ugly but I don't see any way around it. [Except by using a try clause, but I think that would be still uglier.]

I'll update README.md at some point during the next few days.

Are you going to merge the pull request? I seem to have the required permission level to do so but don't want to overstep the mark, especially if you haven't reviewed my code.

All the best.

@rosshemsley
Copy link
Owner

Are you going to merge the pull request? I seem to have the required
permission level to do so but don't want to overstep the mark, especially
if you haven't reviewed my code.

Usual way to do these things is wait until I +1 it, (after review), then
merge.

I'll do that when I get a bit of spare time - Feel free to keep pinging me
if I forget!

On Thu, Dec 17, 2015 at 5:32 PM, mattst [email protected] wrote:

Hi,

Concerning your query about the import:

if python_version_major == 3:
from .matching import ...
from .paths import ...
elif python_version_major == 2:
from matching import ...
from paths import ...

It is due to the new syntax for explicit relative imports introduced in
Python v3.

See the following StackOverflow answer: python: how to import the class
within the same directory or sub directory
http://stackoverflow.com/a/28392732/2102457

However it only seems to be required in packages, as explained at
length on another StackOverflow page, here: Relative imports in Python 3
http://stackoverflow.com/questions/16981921/relative-imports-in-python-3

The code is a bit ugly but I don't see any way around it. [Except by using
a try clause, but I think that will be still uglier.]

I'll update README.md at some point during the next few days.

Are you going to merge the pull request? I seem to have the required
permission level to do so but don't want to overstep the mark, especially
if you haven't reviewed my code.

All the best.


Reply to this email directly or view it on GitHub
#26 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants