Test against package names containing periods
The test rejects any package name containing a period because they will error in Sublime Text 3 if they define python modules.
When the packages are compiled into sublime-package bundles any filenames containing a period confuse the Sublime Text plugin loader. This is on account of python (sub)modules being separated by a period. It attempts to, for example, importlib.import_module('Module.app Menu'). This maps to looking for module "Module" and "app Menu" within for which the sublime-package would be Module.sublime-package but is not where our plugin is packaged to by Package Control.
I'm not sure whether this is the right way to solve this. Arguably Package Control or Sublime should handle the scenario I'm testing against here. Some examples of troublesome packages (you'll see an ImportError in the console when you load Sublime Text 3):
- Marked.app Menu (rename PR: #2525)
- Mou.app Menu
Currently failing tests, which do not all include python modules and therefore do not experience this error:
$ python tests/test.py 2>&1 | grep AssertionError
AssertionError: True is not false : Package names should not contain .: 'Apiary.io Blueprint'
AssertionError: True is not false : Package names should not contain .: 'Asp.Net File Switch'
AssertionError: True is not false : Package names should not contain .: 'Backbone.Marionette'
AssertionError: True is not false : Package names should not contain .: 'Backbone.js'
AssertionError: True is not false : Package names should not contain .: 'Chaplin.js'
AssertionError: True is not false : Package names should not contain .: 'Codepad.org Paste Plugin'
AssertionError: True is not false : Package names should not contain .: 'Conky.tmLanguage'
AssertionError: True is not false : Package names should not contain .: 'Dust.js'
AssertionError: True is not false : Package names should not contain .: 'Ember.js Snippets'
AssertionError: True is not false : Package names should not contain .: 'FakeImg.pl Image Placeholder Snippet'
AssertionError: True is not false : Package names should not contain .: 'jQuery Mobile 1.4 Snippets'
AssertionError: True is not false : Package names should not contain .: 'Kohana 2.x Snippets'
AssertionError: True is not false : Package names should not contain .: 'Lazy Backbone.js'
AssertionError: True is not false : Package names should not contain .: 'Marked.app Menu'
AssertionError: True is not false : Package names should not contain .: 'Mou.app Markdown'
AssertionError: True is not false : Package names should not contain .: 'objc .strings syntax language'
AssertionError: True is not false : Package names should not contain .: 'PEG.js'
AssertionError: True is not false : Package names should not contain .: 'Paste to friendpaste.com'
AssertionError: True is not false : Package names should not contain .: 'Paste to paste.pm'
AssertionError: True is not false : Package names should not contain .: 'Placehold.it Image Tag Generator'
AssertionError: True is not false : Package names should not contain .: 'Play 2.0'
AssertionError: True is not false : Package names should not contain .: 'Require Node.js Modules Helper'
AssertionError: True is not false : Package names should not contain .: 'Ruby 1.9 Hash Converter'
AssertionError: True is not false : Package names should not contain .: 'Simple Ember.js Navigator'
AssertionError: True is not false : Package names should not contain .: 'Snipt.net'
AssertionError: True is not false : Package names should not contain .: 'SourceTree.app Menu'
AssertionError: True is not false : Package names should not contain .: 'Theme - itg.flat'
AssertionError: True is not false : Package names should not contain .: 'Three.js Autocomplete'
AssertionError: True is not false : Package names should not contain .: 'Todo.txt Syntax'
AssertionError: True is not false : Package names should not contain .: 'Underscore.js Snippets'
AssertionError: True is not false : Package names should not contain .: 'wpseek.com WordPress Function Lookup'
Good points. I actually thought at one point that packages could still be imported using importlib if they contained a dot, but I didn't test it and it seems I was wrong.
However, I don't think merging this is the right thing to do (right now), since the names with dots aguably make sense a lot and don't have an equally meaningful alternative (e.g. jQuery Mobile 1.4 Snippets). So, what probably needs to be done is a tweak in ST's internal plugin loading mechanics to circumvent issues with dotted filenames and at least make them able to be imported via importlib, since you have to use that for names with spaces anyway.
I agree that rejecting names on the basis of their including a period isn't the right thing to do. There's three options that could be looked at, then:
- Sublime Text 3 can better handle periods in sublime-package files. There's a little more involved in this than simply looking for the correct filename, as I may have allured to in my initial post description. I'm not sure that this is the correct course of action: there'd be an unacceptable amount of pussy-footing around Python's assumptions/conventions of fully-qualified (dotted) package names elsewhere in the code, potentially.
- Package Control avoids using sublime-package archives for packages with periods in the name. This would be fairly easy to implement -- I understand there's already a similar mechanism where the repository contains a .no-sublime-package. (Based on skuroda's post.)
- Package Control creates sublime-package files with sanitised filenames. For example, "Marked.app Menu" might be stored in the file "Marked_app Menu.sublime-package". This could introduce the possibility of overlapping sublime-package filenames if not handled correctly, but there are easy strategies to avoid such problems; e.g. replace
_with__and.with_.
I think both of the Package Control-side changes will require some careful thought about what's going to happen to the archives that have already been installed by people. Assuming they're equally troublesome, I prefer the last of the three options.
I guess I missed this.
As an author of one of these said packages, Chaplin.js, I'm more than willing to rename.
We have one more option: Ask each author to rename. The list is small enough that it shouldn't be too hard to get the majority on board.
-
Yes, exactly. I think checking if the fully dotted name exists in a plugin directory and then stepping down by each dot split should be sufficient. Alternatively the other way. Possible conflicts in one way or another:
com,com.org,com.org.netpackages.I don't think this has to be supported in anything that is not a packages directory. Package developers should ensure themselves that their subpackages are importable since they will likely need them anyway.
-
Why should the situation change if Package Control unzips the
.sublime-packagefiles? Their paths would still contain dots and remain unable to be imported. -
This could work, but since packages sometimes require hard references in their files to point to certain resouces (syntax files, setting file from within the menu ...), so some of the packages that we "renamed" or some if their features eventually just stopped working. Furthermore, it creates uncertainty for package developers that want to name their package with a . or _ in it because the actual local save location would be different. I'd rather like dots to not be allowed at all than renaming them behind the scenes.
Pinging @wbond~
I just spent a bunch of time trying to see if we can work around importing packages with a . in the name. It isn't really feasible. There are issues the using the imp module for imports related to non-ascii file paths. I know Jon spent a bunch of time dealing with that in the past.
I'm thinking this issue should be resolve by:
- Adding a note to the instructions to tell developers not to name their package with a
.if it uses Python and they want it to work on ST3 - Mark any existing packages with Python that don't work in ST3 and ST2 only compatible
We can let the individual package maintainers decide if they want to deal with the renaming issue.
- Yes, we should definitely do this.
- I'm not sure I understand that, but I think you meant to mark packages that include Python code and are marked as ST3-compatible to be ST2-only, correct?
It might be best to just restrict the use of . all together. Someone might not remember that they cannot have a python file if their repo name has a . in it, and later include some python in the package.
It might be best to just restrict the use of
.all together.
There would certainly be a lot less room for confusion.
I'm okay with restricting package names to not include dots, but we need to rename all packages that currently contain them or rather make the developers rename these (because renaming silently is not good). I don't feel like telling them about this myself tbh.
If @wbond gives the okay. I'll do the leg work.
@wbond what's the verdict?
Since February we've had in the docs instructions not to use . in package names. See https://github.com/wbond/sublime.wbond.net/commit/1ab20e3c61fd1c5af85bde4fd43384042866a187 for reference.
We should notify all package maintainers with a . that they have a week to pick a new name, or we will mark the package only compatible with ST2. Then we can tweak the tests to not worry about the . test if the package is marked for ST2 only.
Here are the packages that advertise ST3 compatibility and have a . in the name:
Play 2.0 https://github.com/guillaumebort/play2-sublimetext2
objc .strings syntax language https://github.com/PaNaVTEC/Objc-Strings-Syntax-Language
Codepad.org Paste Plugin https://github.com/dangelov/Paste-Codepad.org
Dust.js https://github.com/sntran/sublime-dustjs
Animate.css Snippets https://github.com/adeniszczyc/Animate.css-Snippets
Backbone.js https://github.com/tomasztunik/Sublime-Text-2-Backbone.js-package
Theme - itg.flat https://github.com/itsthatguy/theme-itg-flat
Todo.txt Syntax https://github.com/dertuxmalwieder/SublimeTodoTxt
D3.js Snippets https://github.com/fabriciotav/d3-snippets-for-sublime-text-2
Kohana 2.x Snippets https://github.com/golf3gtiii/Kohana234-sublimeText2-plugin
Mou.app Markdown https://github.com/rwoody/mou-markdown-sublime
FakeImg.pl Image Placeholder Snippet https://github.com/tinacious/fakeimg.sublime-snippet
sublimetext.github.com https://github.com/SublimeText/sublimetext.github.com
ApacheConf.tmLanguage https://github.com/colinta/sublime_packages
Chaplin.js https://github.com/joneshf/sublime-chaplinjs
jQuery Mobile 1.4 Snippets https://github.com/MobPro/Sublime-jQuery-Mobile-Snippets
Three.js Autocomplete https://github.com/blackjk3/threejs-sublime
Lazy Backbone.js https://github.com/pwhisenhunt/Sublime-Text-2-Lazy-Backbone.js-Package
PEG.js https://github.com/alexstrat/PEGjs.tmbundle
Ember.js Snippets https://github.com/fabriciotav/ember-snippets-for-sublime-text-2
Backbone.Marionette https://github.com/amokan/ST2-Backbone.Marionette
Conky.tmLanguage https://github.com/oliverseal/Conky.tmLanguage
Require Node.js Modules Helper https://github.com/jfromaniello/sublime-node-require
Underscore.js Snippets https://github.com/carlo/sublime-underscorejs-snippets
wpseek.com WordPress Developer Assistant https://github.com/AlphawolfWMP/sublime-text-2-wpseek
Will a mention from this issue be enough or should we create an issue for each repo? I prefer the first, considering that you get email notifications by default.
We still are going to merge this? Can we start counting the 14 days time out today, and after that, either close this or set those packages as Sublime Text 2 supported only?
@FichteFoll, I reviewed every package on the list @wbond posted and opened a issue on each repository:
- Some repositories where moved and I followed their link change.
- Some repositories where not found on GitHub, but they still present on the
channel.json - Some repositories where already renamed removing the dot
.on the name
Not found
-
[ ] 1) Codepad.org Paste Plugin i. (Page not Found) Perhaps remove it from package control? ii. https://github.com/dangelov/Paste-Codepad.org
-
[ ] 2) objc .strings syntax language i. (Page not Found) Perhaps remove it from package control? ii. https://github.com/PaNaVTEC/Objc-Strings-Syntax-Language
-
[ ] 3) Mou.app Markdown i. (Page not Found) Perhaps remove it from package control? ii. Already renamed to
Mou Markdown App (OSX)on 235c73a01712dab59f9f84c257df705cd3b1c1d6 iii. https://github.com/rwoody/mou-markdown-sublime
Removed from the channel
- [x] 1) sublimetext.github.com i. Removed from the channel on 4e8a465c38ac161c345408484f17d3cb262b01e6 ii. https://github.com/SublimeText/sublimetext.github.com
Already renamed
-
[x] 1) Require Node.js Modules Helper i. Already renamed to
Require CommonJS Modules Helperon 8a8659ce9d46ddc69abfe53bcc156954165c3985 ii. https://github.com/jfromaniello/sublime-node-require -
[x] 2) Dust.js i. Already renamed to
DustBusteron 7b772d11bb33e951318adcb350c11a5e688eb842 ii. https://github.com/sntran/sublime-dustjs -
[x] 3) ApacheConf.tmLanguage i. Issue opened on the original repository asking to rename ii. https://github.com/colinta/ApacheConf.tmLanguage/issues/20 Remove the dot
.on the package nameApacheConf.tmLanguageiii. Already renamed toApacheConfon https://github.com/colinta/sublime_packages/commit/e41b0eb78ae755b218bc9051cc7cd5957f1708e1
Renaming pending
-
[ ] 1) Play 2.0 i. Issue opened on the original repository asking to rename ii. https://github.com/guillaumebort/play2-sublimetext2/issues/32 Remove the dot
.on the package namePlay 2.0 -
[ ] 2) Animate.css Snippets i. Issue opened on the original repository asking to rename ii. https://github.com/adeniszczyc/Animate.css-Snippets/issues/3 Remove the dot
.on the package nameAnimate.css Snippets -
[x] 3) Backbone.js i. Issue opened on the original repository asking to rename ii. https://github.com/tomasztunik/Sublime-Text-2-Backbone.js-package/issues/4 Remove the dot
.on the package nameBackbone.js -
[ ] 4) Theme - itg.flat i. Issue opened on the original repository asking to rename ii. https://github.com/itsthatguy/theme-itg-flat/issues/91 Remove the dot
.on the package nameTheme - itg.flat -
[x] 5) Todo.txt Syntax i. Issue opened on the original repository asking to rename ii. https://github.com/dertuxmalwieder/SublimeTodoTxt/issues/23 Remove the dot
.on the package nameTodo.txt Syntax -
[ ] 6) D3.js Snippets i. Issue opened on the original repository asking to rename ii. https://github.com/fabriciotav/d3-snippets-for-sublime-text-2/issues/1 Remove the dot
.on the package nameD3.js Snippets -
[ ] 7) Kohana 2.x Snippets i. Issue opened on the original repository asking to rename ii. https://github.com/golf3gtiii/Kohana234-sublimeText2-plugin/issues/1 Remove the dot
.on the package nameKohana 2.x Snippets -
[ ] 8) FakeImg.pl Image Placeholder Snippet i. Issue opened on the original repository asking to rename ii. https://github.com/tinacious/fakeimg.sublime-snippet/issues/2 Remove the dot
.on the package nameFakeImg.pl Image Placeholder Snippet -
[ ] 9) Chaplin.js i. Issue opened on the original repository asking to rename ii. https://github.com/joneshf/sublime-chaplinjs/issues/1 Remove the dot
.on the package nameChaplin.js -
[x] 10) jQuery Mobile 1.4 Snippets i. Issue opened on the original repository asking to rename ii. https://github.com/MobPro/Sublime-jQuery-Mobile-Snippets/issues/1 Remove the dot
.on the package namejQuery Mobile 1.4 Snippets -
[ ] 11) Three.js Autocomplete i. Issue opened on the original repository asking to rename ii. https://github.com/blackjk3/threejs-sublime/issues/4 Remove the dot
.on the package nameThree.js Autocomplete -
[ ] 12) Lazy Backbone.js i. Issue opened on the original repository asking to rename ii. https://github.com/pwhisenhunt/Sublime-Text-2-Lazy-Backbone.js-Package/issues/1 Remove the dot
.on the package nameLazy Backbone.js -
[ ] 13) PEG.js i. Issue opened on the original repository asking to rename ii. https://github.com/alexstrat/PEGjs.tmbundle/issues/3 Remove the dot
.on the package namePEG.js -
[ ] 14) Ember.js Snippets i. Issue opened on the original repository asking to rename ii. https://github.com/fabriciotav/ember-snippets-for-sublime-text-2/issues/4 Remove the dot
.on the package nameEmber.js Snippets -
[ ] 15) Backbone.Marionette i. Issue opened on the original repository asking to rename ii. https://github.com/amokan/ST2-Backbone.Marionette/issues/3 Remove the dot
.on the package nameBackbone.Marionette -
[ ] 16) Conky.tmLanguage i. Issue opened on the original repository asking to rename ii. https://github.com/oliverseal/Conky.tmLanguage/issues/4 Remove the dot
.on the package nameConky.tmLanguage -
[ ] 17) Underscore.js Snippets i. Issue opened on the original repository asking to rename ii. https://github.com/carlo/sublime-underscorejs-snippets/issues/8 Remove the dot
.on the package nameUnderscore.js Snippets -
[x] 18) wpseek.com WordPress Developer Assistant i. Issue opened on the original repository asking to rename ii. https://github.com/wpseek/sublime-text-2-wpseek/issues/6 Remove the dot
.on the package namewpseek.com WordPress Developer Assistant
Thanks for taking the initiative for this! I'll try to keep that list updated when I merge changes to the channel.
We are probably going to prune a couple missing packages at some point in the future. For this PR, we can just remove the offending and deleted packages when the others have been renamed.
I'd rather give people a one month period before they respond and force-rename their packages when they haven't. The reason why declaring ST2 compatibility won't work is that I we want to test all repositories for not containing dots.
Why not add some code - if an author includes a decimal, when PackageControl handles the installation, etc... Modify all files with import * and change the . to an _ and let the author know, and output the information in the PackageControl log..
This way, existing plugins with incorrect naming can still be used - but it requires some preprocessing of the package each time an update occurs, etc...
While it is possible, I would say it is too much overhead to handle. I my opinion, there is not urgent need to allow packages use dots on their names, and if their authors are not available to simply rename the package, I would say the package is more likely to be unmaintained/abandoned, then, there is not much need for they to be on Package Control.
Not worth it, imo. I say we just rename all offending packages and prune the missing ones. Not sure when I'll get to that, though.
Could this be given more visibility ( documentation update would be great ), to give a heads-up to future package writer ?
This is old hat by now. Nothing mentioned here is still actionable. The docs already very clearly cover invalid characters for package names and it hasn’t been an issue for years. The only real issue was the change from ST2 to ST3, but we’re at ST4 already (and for several years). Packages that stopped working have either already been fixed or abandoned. Let’s leave it at that.