ternjs / tern_for_sublime

Sublime Text package adding Tern support
MIT License
803 stars 54 forks source link

Use CSS templates and proper HTML inrenderer. get_description_message #125

Closed pdanpdan closed 7 years ago

marijnh commented 8 years ago

I'm okay with providing some way for users to specify, for example, the path to a CSS file, or even some inline CSS in their options. But having our own copy of a plist parser in here + some custom config format that is then converted to CSS, that doesn't seem like a good idea.

(@tansongyang Do you agree?)

tansongyang commented 7 years ago

@pdanpdan You did some cool work here. It's a very nice effect.

I briefly looked through the code and it seems that the plistparser is there in order to parse the styles for the current theme so that we can make the tooltip match. Would it be possible to include this as a dependency or something like that, so that we don't have to have our own implementation? Having our own feels like unneeded clutter.

Other than that, @marijnh , this looks pretty good to me. The "custom config format" you mention is, I think, just how Sublime Text stores themes.

tansongyang commented 7 years ago

@pdanpdan Does this work consistently for you? I tried to load up Sublime again this morning and it appears that the styling on the tooltips have gone away. I had switched to a new theme. Switching back doesn't appear to fix it.

pdanpdan commented 7 years ago

@tansongyang For me it's working just fine. Sublime 3119 ubuntu64, Boxy Monokai theme, gruvbox (Dark) (Medium) color scheme, mostly for javascript. But I cannot take credit for the main changes - they were done by FreaKzero .

Jastrzebowski commented 7 years ago

@tansongyang @pdanpdan for me works great with some custom thems but crashes with defaults one:

Traceback (most recent call last):
  File "/opt/homebrew-cask/Caskroom/sublime-text3/Build 3083/Sublime Text.app/Contents/MacOS/sublime_plugin.py", line 320, in on_selection_modified_async
    callback.on_selection_modified_async(v)
  File "/Users/koala/Library/Application Support/Sublime Text 3/Packages/tern_for_sublime/tern.py", line 53, in on_selection_modified_async
    on_selection_modified(view)
  File "/Users/koala/Library/Application Support/Sublime Text 3/Packages/tern_for_sublime/tern.py", line 33, in on_selection_modified
    if pfile is not None: show_argument_hints(pfile, view)
  File "/Users/koala/Library/Application Support/Sublime Text 3/Packages/tern_for_sublime/tern.py", line 463, in show_argument_hints
    if call_start is None: return render_argument_hints(pfile, view, None, 0)
  File "/Users/koala/Library/Application Support/Sublime Text 3/Packages/tern_for_sublime/tern.py", line 478, in render_argument_hints
    renderer.clean(pfile, view)
AttributeError: 'NoneType' object has no attribute 'clean'
marijnh commented 7 years ago

@tansongyang

The "custom config format" you mention is, I think, just how Sublime Text stores themes.

Really, copy/pasting a plist parser is the recommended way to write Sublime Text plugins that handle themes?

tansongyang commented 7 years ago

@marijnh Looking back, I think I misread your comment, so you can disregard that.

I don't know what the best way is. I took a brief look at some other plugins that I know do something with the current color scheme:

  1. Color Highlighter uses string manipulation. Doesn't seem ideal.
  2. Sublime Linter uses ElementTree, which seems better to me.

I also discovered that Python comes with plistlib, so maybe it would be best to use that. In any case, it looks like the plugin will have to do some parsing if it's going to adjust to the user's color scheme.

marijnh commented 7 years ago

Is it not an option to directly allow the user to provide CSS? That's what it looks like we eventually need anyway, and it's a language that people tend to be familiar with.

tansongyang commented 7 years ago

@marijnh Yes, it's certainly an option to let the user provide CSS. In my comments, I was discussing the route where the plugin tries to detect the colors in the color scheme and automatically match. As I see it, there are 2 routes:

  1. Let user provide CSS, even just color values.
  2. Try to be smart and figure out what colors the current color scheme is using, which is what's going on here.
tansongyang commented 7 years ago

@marijnh Yes, it's certainly an option to let the user provide CSS. In my comments, I was discussing the route where the plugin tries to detect the colors in the color scheme and automatically match. As I see it, there are 2 routes:

  1. Let user provide CSS, even just color values.
  2. Try to be smart and figure out what colors the current color scheme is using, as is happening here.
marijnh commented 7 years ago

Closing this, as the target branch seems to be used for other stuff now.