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

WIP: Getting folding right in Elpy #1398

Merged
merged 5 commits into from
Nov 14, 2019

Conversation

galaunay
Copy link
Collaborator

@galaunay galaunay commented Jun 19, 2018

PR Summary

Follow (old) feature request #240.

This is an attempt to get nice folding functionalities in Elpy.
It is currently based on hs-minor-mode and heavily inspired from hideshowvis.

Any idea is welcome.

Todo list

Nice folding

  • Add an overlay indicating the number of folded lines.
  • Add a fringe indicator for folded code.
  • Make fringe indicators clickable
  • Make folding indicators clickable
  • Add fringe indicators to mark the beginning of foldable regions.
  • Add fringe indicator for dosctrings ?

Folding docstrings

  • Add elpy-folding-toggle-docstrings to hide all the docstrings in the buffer.
  • Add elpy-folding-hide-docstring-at-point functions to hide the docstring at point.
  • Find a way to show the first docstring line when folding docstring.

Folding comments

  • Add a function to hide all comments

DWIM

  • Add a function to toggle folding dor the thing (block, docstring, region or comment) at point

Selective folding

  • Add elpy-folding-hide-leafs to hide blocks not containing other blocks.

Keybindings

  • Add elpy-folding-fold-at-point to fold the active region, the docstring, or the block at point.
  • Bind elpy-folding-toggle-at-point (to C-c @ p).
  • Bind elpy-folding-toggle-all-docstrings (to C-c @ b).
  • Bind elpy-folding-hide-leafs (to C-c @ f).

Other modules incompatibilities

  • Put flymake fringe indicators on the right fringe when folding module is activated to avoid fringe indicator overlap.

Others

  • Clean and reorganize properly
  • Write tests
  • Update the documentation

@coveralls
Copy link

coveralls commented Jun 19, 2018

Coverage Status

Coverage decreased (-0.01%) to 92.021% when pulling 08911cb on galaunay:Folding-module into d54e78e on jorgenschaefer:master.

@galaunay
Copy link
Collaborator Author

@jorgenschaefer, before I begin to add tests and documentation, do you have any feedback on this ?

Notably, I am not sure about the keybindings.

Also, It is inspired from here for the leafs hiding and hideshowvis (GPL v2) for the fringe and folding overlays. Not sure if we need to add that as source somewhere.

@jorgenschaefer
Copy link
Owner

This looks really nice. The only general feedback I have is "are you sure you want to maintain this in the future" – any such larger code base will need maintenance to keep up to date with dependent packages. But other than that, nice.

The key bindings are pretty standard for Emacs. I never liked C-c @, because @ is "right alt + q" on German keyboards, but that's a general problem. Being standard, I think this is fine.

Thank you again for all the great work you do!

@galaunay
Copy link
Collaborator Author

galaunay commented Jul 3, 2018

This is a good advice, I will try to keep it simple.
Notably, I am not sure that mouse integration is really necessary.

I'll stick to the standard key bindings then (not really convenient on the french keyboard as well, but I'll rebind that).

@peterewills
Copy link

Thanks for building this! 🙏 I've wanted code folding in elpy for a while, and now I'm running your branch and it works well. Would be nice to have this in master!

@galaunay
Copy link
Collaborator Author

Thanks for the feedback.
I kept this PR on the side because of lack of time for proper testing.
Apart of that, I think it is kind of ready to be merged.

So If you are saying it's working, I could give it a quick test and merge it.
Any additional features you would find useful ?

@peterewills
Copy link

peterewills commented Oct 13, 2019

I've actually been having some weird issues with hs-hide-level. These don't appear when I just use hs-minor-mode on top of the MELPA version of elpy.

Essentially, when I run hs-hide-level 2, it only hides every other block at level 2. This only appears on my (larger & more complicated) work files, which I can't share here, so I'll have to take some time to try and build a minimal example. I made a very simple .py file with a number of levels, and that seems to work fine, so it's something in the way that things get parsed on larger files with docstrings, comments, etc.

I'll pull down this branch at the latest commit and make sure I'm still seeing the bug, and then try to get a minimal example up here.

As for features: The only thing that I want is a function that does "hide all blocks that are either a leaf or at level n", which is like elpy-folding-hide-leafs but won't expand any blocks beyond level n. That's kind of idiosyncratic to my preferences, though. Do you think it's worth including here? I'm thinking I'll write up that functionality and then submit at PR to your branch.

UPDATE: When I run from 533eb6b, I'm not seeing these issues - might have been some weird fluke. However, I am having trouble with elpy-folding-hide-leafs; it throws Lisp nesting exceeds ‘max-lisp-eval-depth’.

elpy.el Outdated
(when (hs-find-block-beginning)
(setq beg (1+ (point)))
(funcall hs-forward-sexp-func 1)
(setq end (1- (point))))

Choose a reason for hiding this comment

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

I think this needs to move the pointer back to the beginning of the block here.

Suggested change
(setq end (1- (point))))
(setq end (1- (point)))
(goto-char beg))

@galaunay galaunay merged commit f3461ee into jorgenschaefer:master Nov 14, 2019
@galaunay galaunay deleted the Folding-module branch November 14, 2019 22:14
@galaunay
Copy link
Collaborator Author

@peterewills I finally managed to finish and merge this PR.

The folding module is deactivated by default, you will have to activate it trough customize or by adding it to elpy-modules.
Hope you will find it useful.

@azzamsa
Copy link

azzamsa commented Nov 14, 2019

instead of having independent module for folding, why don't we just tell user to use external package such us origami https://github.com/gregsexton/origami.el ?

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.

None yet

5 participants