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

Add plugin: pycalc #9017

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

Add plugin: pycalc #9017

wants to merge 2 commits into from

Conversation

pycalc-plugin
Copy link

@pycalc-plugin pycalc-plugin commented Dec 5, 2024

  • I'm the package's author and/or maintainer.
  • I have have read the docs.
  • I have tagged a release with a semver version number.
  • My package repo has a description and a README describing what it's for and how to use it.
  • My package doesn't add context menu entries. *
  • My package doesn't add key bindings. **
  • Any commands are available via the command palette.
  • Preferences and keybindings (if any) are listed in the menu and the command palette, and open in split view.

My package is pycalc https://github.com/pycalc-plugin/sublime

There are no packages like it in Package Control.

@pycalc-plugin pycalc-plugin changed the title Update p.json Add plugin: pycalc Dec 5, 2024
@kaste
Copy link
Contributor

kaste commented Dec 9, 2024

Hi! I initially thought "yeah" but then "why not just open a repl or ipython in Terminus like I do right now". Generally your approach to the problem is kind of obtrusive, e.g. you grab the enter key for basically every view, and you use stderr and stdout. This might be just the first lazy, drafty approach but you need proper implementations to pass the review here.

Besides, why do you want a public plugin but hide yourself? Who are you when the commit email you used is "user <[email protected]>"?

@pycalc-plugin
Copy link
Author

Hello @kaste

When you close the repl or console you will lose the code and data (or have to copy and clean from >>>)

In my solution the code and data are stored in a simple text file, it borrows the philosophy of jupyter notebook simplifying it as much as possible and makes it accessible to any editor that supports plugins (I'm working on it, see https://github.com/pycalc-plugin)

It seems intrusive, but you can always disable the plugin from the context menu, the user decides whether to use it or not.

publishing email = spam, you can always write to me in github issues.

Best regards,
pycalc

@kaste
Copy link
Contributor

kaste commented Dec 11, 2024

Well, you bind enter for every view as I understand your implementation. That's not a good-enough approach imo. Why is that useful or necessary? My expectation would be that the user initiates "Give me a new scratch buffer which is a REPL" or "Convert this current view to a REPL". All other views shouldn't be touched. Maybe the is_enabled flag should a view setting instead of a global switch.

You should also implement InteractiveConsole so that it doesn't use stdout/err.

@braver
Copy link
Collaborator

braver commented Dec 13, 2024

That's not a good-enough approach imo

Indeed it really isn't.

I can see at least 2 alternatives:

  • Either the user enters a mode (for a particular view, not globally) where it makes sense for return to be captured this way
  • or you don't bind a key at all and instead just allow the user to run a command any way they like

Additionally:

  • allowing you to run pycalc on a selection is handy, but not via the context menu (the user needs control over what appears in there); at the very least add that to the command palette
  • taking up a content menu item just to toggle your plugin is also not ideal
  • check the CI output
  • where did halve the checkboxes in the checklist go?
  • "There are no packages like it in Package Control" -> there seem to be a couple?
  • If you put a description on the Github repo, you don't need on in your entry here

@pycalc-plugin
Copy link
Author

pycalc-plugin commented Dec 15, 2024

Well, you bind enter for every view as I understand your implementation. That's not a good-enough approach imo.

I don't know another way to catch the enter key press, if you know another way, please tell me.

Maybe the is_enabled flag should a view setting instead of a global switch.

What do you mean, how to do it?

You should also implement InteractiveConsole so that it doesn't use stdout/err.

I don't touch the global interpreter, I create a local InteractiveConsole and using the redirect_stdout, redirect_stderr functions I get the result for output to sublime.

allowing you to run pycalc on a selection is handy, but not via the context menu (the user needs control over what appears in there); at the very least add that to the command palette

I can add an item to the command palette that will hide context menu items at the user's request.

check the CI output

Done

where did halve the checkboxes in the checklist go?

I have removed items that are not related to my plugin, if necessary I can return them.

"There are no packages like it in Package Control" -> there seem to be a couple?

I have looked through and installed these plugins but have not found one that is as convenient and powerful.

p.s. To help you understand my logic, I'll add a screencast of the user experience with the plugin.

screenshot

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: pycalc
Results help

Packages added:
  - pycalc

Processing package "pycalc"
  - WARNING: The binding ['enter'] is also defined in default bindings but is masked with a 'context'
    - File: Default.sublime-keymap

@braver
Copy link
Collaborator

braver commented Dec 15, 2024

I don't know another way to catch the enter key press

The problem is that you're catching all key presses everywhere (except widgets). You want to try and only listen to keypresses in your specific "repl-like" views.

You could try to ensure such a view has a specific syntax, and use that as a context for the keybinding. You could then also use that syntax to differentiate input and output.

Also note that views have a settings interface you can manipulate.

if necessary I can return them.

Yes please, I can't tell which one's you skipped if they're not there. Maybe they're not applicable, but let me worry about that 😉

I can add an item to the command palette that will hide context menu items at the user's request.

The way you're doing might not work. Your package directory is not writable as your package is delivered in a zipped format.

If you manage where your event listener is active, you don't need an on/off toggle. That leaves you with one command in the context menu. It would be cleaner if you introduce a setting that toggles the that entry via an is_visible() check.

@kaste
Copy link
Contributor

kaste commented Dec 15, 2024

I don't know another way to catch the enter key press, if you know another way, please tell me.

If it is a skill issue then the Discord channel might be interesting for you. http://discord.sublimetext.io/ https://discord.com/channels/280102180189634562/280157067396775936

One approach is to have a key binding with a context like this:

{ "key": "setting.this_is_a_pycalc_repl" }

Then you just need a way to toggle this setting on and off. E.g. you create a window command and create a new view and set it. That looks like:

class pycalc_new_replp(sublime_plugin.WindowCommand):
    def run(self):
        new_view = self.window.new_file(syntax="source.python")
        new_view.settings().set("this_is_a_pycalc_repl", True)

Refer the API documentation https://www.sublimetext.com/docs/api_reference.html#sublime.Window.new_file

Add this command to the Command Palette, and you probably don't need any context menu handlers or settings. (At least for the proof-of-concept.)

The trickier part may be:

I don't touch the global interpreter, I create a local InteractiveConsole and using the redirect_stdout, redirect_stderr functions I get the result for output to sublime.

The shared resources are stdout and stderr.

IIRC you can implement InteractiveConsole (that is: subclass it) as it has some methods you need to implement so that it works with your own (likely: StringIO?) streams.

Very likely the module is well documented and you can find tutorials for this as well.

@kaste
Copy link
Contributor

kaste commented Dec 15, 2024

A very similar plugin is btw https://github.com/ichichikin/sublime-plugin-interactivity

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

Successfully merging this pull request may close these issues.

4 participants