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

Fix behavior of paragraph hopping key bindings. #147

Closed
wants to merge 1 commit into from

Conversation

jart
Copy link

@jart jart commented Jul 3, 2014

The default emacs behavior for the forward-paragraph and
backward-paragraph functions is to hop between blank lines in a source
code file. This breaks in js2-mode in the presence of javadoc
@clauses.

This commit fixes this behavior by adding advice to the following
standard emacs functions, which only applies to js2-mode:

  • forward-paragraph
  • backward-paragraph
  • fill-paragraph

The existing functionality for filling paragraphs within javadocs is
preserved.

Video explanation: http://youtu.be/cXerimuxNK8

The default emacs behavior for the `forward-paragraph` and
`backward-paragraph` functions is to hop between blank lines in a source
code file. This breaks in `js2-mode` in the presence of javadoc
@clauses.

This commit fixes this behavior by adding advice to the following
standard emacs functions, which only applies to `js2-mode`:

- forward-paragraph
- backward-paragraph
- fill-paragraph

The existing functionality for filling paragraphs within javadocs is
preserved.

Video explanation: <http://youtu.be/cXerimuxNK8>
@dgutov
Copy link
Collaborator

dgutov commented Jul 5, 2014

Hi Justine,

I realize the implementation took some effort, but I'm against merging this for a few reasons:

  • I'm not sure this is the right thing to do. The fact that the three -paragraph- commands share the same notions of where the paragraphs start and end, makes sense. Making one of them work on different paragraphs will be surprising. I don't know how many people use fill-paragraph in comments with js2-mode, but some of them probably expect backward-paragraph not to jump over jsdoc comments.

  • This is most likely the wrong way to do it. The presence of six defadvice forms is a big hint. Since you only want fill-paragraph to work differently from the rest, it would be easier to set js2-paragraph-start to \n\n, and then either define js2-mode-specific version of fill-paragraph that would bind the required vars locally and then call fill-paragraph, or write a special fill-forward-paragraph-function (see that var's description) that would take jsdoc clauses into account.

    This question has never come up before, and I wasn't anywhere near when js2-paragraph-start definition was written, so if that approach appeals to you, you should M-x report-emacs-bug and argue the case to the Emacs core developers (they can commit the patch to GNU ELPA, and I'll propagate it here). They might have a more informed opinion on the subject.

    Or you can write a couple of functions, or a new minor mode, that do the same. Maybe publish it for similar-minded users.

  • All legally significant contributions must be accompanied with copyright assignment to FSF. I really should have added this to the README long time ago, sorry.

@dgutov dgutov closed this Jul 5, 2014
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

2 participants