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

Handle file-level docstrings in prog-mode #78

Merged
merged 1 commit into from
Sep 2, 2023

Conversation

doolio
Copy link
Contributor

@doolio doolio commented Sep 1, 2023

Closes #77.

This seems to work in a python file with a file level docstring.

As you'll see I really just adapted the existing beginend--point-is-in-comment-p (private) function. Not sure if I fully understand the let form yet so it may not be necessary in beginend--point-is-in-string-p. Plus you may not like this solution as it probably goes against the principles of the DRY methodology.

Not entirely sure how I should run the tests. I executed buttercup-run-at-point at the top level describe form in beginend-prog-test.el which resulted in the following output. Perhaps buttercup should have a buttercup-run-on-buffer type command as well. The individual test times are interesting. I added the new "for a docstring" test.

Running 9 specs.

beginend
  in prog-mode
    uses prog-mode-code-position-p to know where code begins (0.57ms)
    uses prog-mode-code-position-p to know where code end (0.39ms)
    moves point to beginning of first code line (0.77ms)
    moves point to end of last code line (0.57ms)
  beginend--prog-mode-code-position-p
    returns non-nil for a line of code (3.75ms)
    returns nil
      for a comment line (2.74ms)
      for a docstring (2.64ms)
      for an empty line (3.10ms)
      for a line with ^L (2.81ms)

Ran 9 specs, 0 failed, in 18.51ms.

The buttercup docs only document running tests when using cask, eldev, and eask but I think you use makel.mk right?

Since you use buttercup why do you need the .ert-runner file? Is it because you refer to ${TEST_ERT_FILES} in your Makefile. Excuse my ignorance here.

Finally, I noted the following comment at several places in begined-prog-test.el:

;; workaround for https://github.com/jorgenschaefer/emacs-buttercup/issues/84

But if understand the exchange in that issue then these comments are obsolete as you have put spy-on in a before-each form. I have left them in for now but can remove them if you agree.

Copy link
Owner

@DamienCassou DamienCassou left a comment

Choose a reason for hiding this comment

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

Thank you very much for your great work, I really appreciate.

Not sure if I fully understand the let form yet so it may not be necessary in beginend--point-is-in-string-p. Plus you may not like this solution as it probably goes against the principles of the DRY methodology.

Indeed, there is a lot of duplication but I think this is good enough.

Not entirely sure how I should run the tests.

Usually, you can check how the CI is ran in a project to see how to run the tests. In this case, the CI configuration is in .github/workflows. This will lead you to https://github.com/DamienCassou/emacs-workflows/blob/main/.github/workflows/makel.yml which last line is make check. Anyway, the CI checked your PR already and everything is fine!

I think you use makel.mk right?

Indeed: https://github.com/damienCassou/makel

why do you need the .ert-runner file?

This could be removed, you are right.

I have left them in for now but can remove them if you agree.

This comment should have been deleted some time ago. Feel free to get rid of them.

test/beginend-prog-test.el Show resolved Hide resolved
@DamienCassou DamienCassou merged commit 2d35369 into DamienCassou:master Sep 2, 2023
1 check passed
@DamienCassou
Copy link
Owner

I merged your PR. Feel free to open a new one for the cleanup you found.

@doolio doolio deleted the issue-77 branch September 2, 2023 15:20
@doolio
Copy link
Contributor Author

doolio commented Sep 2, 2023

I think you use makel.mk right?
Indeed: https://github.com/damienCassou/makel

It may not matter but I note your Makefile here is using an older version than your version on github.

@DamienCassou
Copy link
Owner

It may not matter but I note your Makefile here is using an older version than your version on github.

would you mind sending a PR to update makel?

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.

Should beginend consider module docstrings in python buffers?
2 participants