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 python comment #390

Closed
wants to merge 5 commits into from
Closed

Add python comment #390

wants to merge 5 commits into from

Conversation

andrewfowlie
Copy link

This adds a snippet to comment/uncomment Python code with Ctrl+m.

  • If no text selected, toggles comments for current line
  • Preserves leading space in front of comment/text that is uncommented
  • Depends on re (import re)

I think this is a very useful enhancement. It could be quickly generalized to other languges just by changing # to // or whatever, but I'm not sure about how to best achieve that without lots of duplication.

@raveit65
Copy link
Member

No idea about python and i was forced to disable it in fedora, because libpeas droped python2 report.
https://src.fedoraproject.org/cgit/rpms/pluma.git/commit/?h=f29&id=108ab60d283f9ca4489fa2f80872823f791f867c

@raveit65 raveit65 requested review from a team January 16, 2019 14:53
@andrewfowlie
Copy link
Author

Sorry I don’t understand your comment or know anything about libpeas! Let me know if I should change something

@raveit65
Copy link
Member

I only said that i don't use python plugins anymore as fedora maintainer and i am not familiar with python.
libpeas is used to handle plugins in pluma..

@lukefromdc
Copy link
Member

I don't know anything about this, as I don't use any external plugins at all. I see that snippets uses python so should be testable for function but it's not something I normally use so I could miss a subtle problem either in master or after this commit. None the less I have put this on my schedule for testing

@lukefromdc
Copy link
Member

I see that
#389
is included in this

@andrewfowlie
Copy link
Author

Yes, #389 is necessary for the Python plugin to work.

Sorry I’m not sure what the correct procedure is for making a second PR that depends on the first one. I wanted this PR to contain working code so included it here too.

@lukefromdc
Copy link
Member

If this PR is merged first, the first one will need to be closed. If the other one is merged first, this one will need a rebase on master and a forced update, almost certainly including manual resolution of a merge conflict.

@andrewfowlie
Copy link
Author

andrewfowlie commented Jan 18, 2019

I have refactored my contribution to pluma. In my original commits, I added the comment snippet to he Python language in an ad hoc way. In this one, I have refactored the code for toggling comments so that it can be quickly used in snippets for any language. Essentially, you just have to change the comment string for that language - e.g., # for Python, // for C/C++ etc (though at the moment we only have Python one)

Sorry if this has made more work for you! But hopefully the code is now easier to review, re-use, maintain etc.

@andrewfowlie
Copy link
Author

Actually I’ve just seen how I can very quickly generalise this to all languages. Will add more commits soon

@raveit65
Copy link
Member

#389 is merged, you can rebase PR against master.
git rebase master

@raveit65
Copy link
Member

raveit65 commented Jun 6, 2019

@andrewfowlie
Can you rebase PR against master please?
We are using python3 now.

@vkareh
Copy link
Member

vkareh commented Jun 7, 2019

@andrewfowlie - this has a conflict, can you fix it? Also, I get this error when enabling the snippets plugin:

(pluma:13297): libpeas-WARNING **: 09:43:12.468: Error importing plugin 'snippets':
Traceback (most recent call last):
  File "/usr/lib/python2.7/importlib/__init__.py", line 37, in import_module
    __import__(name)
  File "/usr/lib/x86_64-linux-gnu/pluma/plugins/snippets/__init__.py", line 24, in <module>
    from comment import toggle_lines
ImportError: No module named comment

(pluma:13297): libpeas-WARNING **: 09:43:12.468: Error loading plugin 'snippets'

Andrew Fowlie added 4 commits June 17, 2019 11:30
@andrewfowlie
Copy link
Author

I have fixed the conflict and rebased.

Regarding the error, it seems that the pluma/plugins/snippets/snippets/comment.py file isn't being copied to the correct directory by the installer. It should go here /usr/lib/x86_64-linux-gnu/pluma/plugins/snippets/ too. If you want to test the code w/o fixing this installation issue, copy it there manually, or

export PYTHONPATH=PATH/TO/pluma/plugins/snippets/snippets/

might also work.

@raveit65 raveit65 requested review from a team and removed request for a team June 17, 2019 05:32
@yetist
Copy link
Member

yetist commented Jun 17, 2019

Lost some files when installing:

(pluma:21149): libpeas-WARNING **: 15:38:12.575: Error importing plugin 'snippets':
Traceback (most recent call last):
  File "/usr/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/lib/pluma/plugins/snippets/__init__.py", line 24, in <module>
    from .comment import toggle_lines
ModuleNotFoundError: No module named 'snippets.comment'

(pluma:21149): libpeas-WARNING **: 15:38:12.575: Error loading plugin 'snippets'

@andrewfowlie
Copy link
Author

@yetist , yes, how to fix?

@yetist
Copy link
Member

yetist commented Jun 17, 2019

You can edit the plugins/snippets/snippets/Makefile.am file, and append your new files to plugin_PYTHON.

@andrewfowlie
Copy link
Author

You can edit the plugins/snippets/snippets/Makefile.am file, and append your new files to plugin_PYTHON.

done

Copy link
Member

@yetist yetist left a comment

Choose a reason for hiding this comment

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

It works.
Press 'Ctrl-M' can toggle comment/uncomment the current line, if select one or more lines, it can do the same things.

@raveit65
Copy link
Member

I builds fine and i can start pluma with activated snippet pluging, But crtl+m does nothing in adocument with python code.
Does this work for someone other?

Beside from that i think commit 3,4,5 are fixes for commit 2, or not ?
Can those commits squashed into 2 please?

Copy link
Member

@vkareh vkareh left a comment

Choose a reason for hiding this comment

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

This works fine for me. As long as you have the Snippets plugin enabled, pressing <Ctrl>+m comments the selected line/block.

@vkareh
Copy link
Member

vkareh commented Jun 26, 2019

I agree with @raveit65 about squashing, I think I would squash the entire thing into a single feature commit

@raveit65
Copy link
Member

Squashed and merged.
8e7e455
Thanks

@raveit65 raveit65 closed this Jun 26, 2019
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

5 participants