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

Distinguish Naomi syntax names from others #162

Merged
merged 1 commit into from
Jul 20, 2018

Conversation

TomasBarry
Copy link
Contributor

No description provided.

@TomasBarry TomasBarry mentioned this pull request Jul 20, 2018
@borela
Copy link
Owner

borela commented Jul 20, 2018

In the beginning I used similar names to the core's syntaxes to make it compatible with other plugins that rely on the syntaxes' names, I'll take a look if it does not break them.

@TomasBarry
Copy link
Contributor Author

Hi @borela, what I've done for a while now is just update the names in the downloaded package but then any update that was pushed to the package overwrote my changes so I thought it'd be best to make a PR for this.

WRT to the failed travis check, you'll have to point me in the right direction in fixing those test errors.

@borela
Copy link
Owner

borela commented Jul 20, 2018

Don't worry about the travis errors, that's not caused by your patch, for the past 3 months I've been updating the scopes to enhance the compatibility with the core syntaxes and it broke many tests.

Your patch is something that I wanted to do for a long time, sometimes people get confused if they are in fact using these syntaxes because the names are too similar. I just have to check some plugins but I believe this change is fine.

@TomasBarry
Copy link
Contributor Author

TomasBarry commented Jul 20, 2018

@borela, sounds good. Fixed one travis issue FWIW anyway in this PR.

Hopefully there are no issues. This is a great project.

@borela borela merged commit 6103324 into borela:master Jul 20, 2018
@borela
Copy link
Owner

borela commented Jul 20, 2018

I tried to use some plugins and this change doesn't seam to affect them. Thank you! :D

@borela
Copy link
Owner

borela commented Jul 20, 2018

Forgot to ask you to write your name and email in one of the licenses, which one do you want?

@TomasBarry
Copy link
Contributor Author

The MIT one sure, will I open another PR for that @borela?

@TomasBarry TomasBarry deleted the distinguish-syntax-names branch July 23, 2018 08:56
@TomasBarry
Copy link
Contributor Author

How long does it typically take before the package is updated on Package Control?

@borela
Copy link
Owner

borela commented Jul 23, 2018

About ~3h max after I tag a commit, don't need to need a new pull request, I'll send a patch to the MIT license. Do you want to add the email on your profile or do you have another one that you prefer?

@StreetStrider
Copy link
Contributor

That was needed indeed. Previously Naomi's sytaxes was indistinguishable from default ones both in status bar (current) and in command palette (when you apply them).

@TomasBarry
Copy link
Contributor Author

tomas.barry@hotmail.com will do just fine @borela, have you tagged the commit yet?

@borela
Copy link
Owner

borela commented Jul 23, 2018

Modified the license and tagged the commit https://github.com/borela/naomi/releases/tag/v4.1.1 considering package control crawled the repo 40 minutes ago, it should update in about 2:30h.

@TomasBarry
Copy link
Contributor Author

Brilliant. Thanks @borela

@StreetStrider
Copy link
Contributor

@borela @TomasBarry after update, my snippets (located under Packages/JavaScript/, and with scope source.js) stop working. How do I inherit them to work for JavaScript (Naomi) as well?

@borela
Copy link
Owner

borela commented Aug 2, 2018

@StreetStrider It should continue to work as expected, try cleaning the cache directories:

  • OS X: ~/Library/Application Support/Sublime Text 3
  • Windows: %LOCALAPPDATA%\Sublime Text 3
  • Linux: ~/.config/sublime-text-3

@borela
Copy link
Owner

borela commented Aug 2, 2018

Another thing I did, I excluded the node_modules directory, I usually have many projects open and that was screwing the indexer for me, this is my settings:

{
	"auto_id_class": true,
	"color_scheme": "Packages/Naomi/Candyman.tmTheme",
	"draw_shadows": false,
	"draw_white_space": "selection",
	"ensure_newline_at_eof_on_save": true,
	"folder_exclude_patterns":
	[
		".svn",
		".git",
		".hg",
		"CVS",
		"*/node_modules"
	],
	"font_face": "Monaco",
	"font_size": 15,
  	"ignored_packages":
	[
		"Six",
		"Vintage",
		"Vintageous"
	],
	"indent_guide_options":
	[
		"draw_active"
	],
	"index_files": false,
	"preview_on_click": false,
	"rulers":
	[
		80,
		120
	],
	"show_encoding": true,
	"show_line_endings": true,
	"skin": "DA UI/Classic",
	"theme": "DA.sublime-theme",
	"translate_tabs_to_spaces": true,
	"trim_trailing_white_space_on_save": true,
	"vintageous_use_ctrl_keys": true,
	"vintageous_use_sys_clipboard": true
}

@StreetStrider
Copy link
Contributor

@borela my issue somehow resolved.

On the offtopic note, are you sure folder_exclude_patterns supports globbing? As far as I used it in my sublime-project settings, it's pretty dumb and does not recognize any nesting/globbing.

@borela
Copy link
Owner

borela commented Aug 4, 2018

@StreetStrider It does but you are correct in the fact that in that case, node_modules and */node_modules have the same effect, I did some tests targeting a folder under another one e.g.: modules/**/foo and the globbing worked as expected.

@StreetStrider
Copy link
Contributor

@borela nice to hear that. I had issues when I got directory release/ with generated files which I want to ignore and src/release or something like, which is legit src files. They all got banned by simple release pattern (obviously), but I tried to improve globbing and it just stops ignoring at all. I will re-iterate this, maybe recent ST3 release finally improved that.

@borela
Copy link
Owner

borela commented Aug 4, 2018

@StreetStrider I see what you mean, you need negation and that is not working as expected, found some issues on the topic :/

sublimehq/sublime_text#1895
sublimehq/sublime_text#1897

@StreetStrider
Copy link
Contributor

@borela, yes, that's the case exactly.

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.

None yet

3 participants