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

Improve the installation process & some minor changes #9

Merged
merged 5 commits into from
Jul 18, 2022

Conversation

SqrtMinusOne
Copy link
Contributor

A bunch of changes to resolve the problems I had with installing this package. Hope you don't mind some refactoring.

First, there's a convention that the prefix of the package has to be the same as the package name, i.e. dank-mode- in this case (maybe in the future someone makes a package called dank-posts that does something totally different). So I changed the names accordingly. You'll most likely be required to do so anyhow if you submit this to MELPA.

This also allows for having one main file dank-mode.el, which lists all the dependencies of the package. Which I've also done, so now the package can be installed with straight.el, sans this issue. I added a paragraph with a workaround to the README.

Also:

  • Enabled lexical binding everywhere. MELPA also requires this.
  • Added a note that the OAuth dance won't work in EXWM
  • Added a variable dank-mode-prefer-new-reddit to prefer reddit.com over old.reddit.com
  • Changed dank-mode-comments-browse-post-link, dank-mode-comments-browse-post-comments, dank-mode-posts-browse-post-link-at-point and dank-mode-posts-browse-post-comments-at-point to set the eww argument with the prefix argument. Otherwise the argument is always nil.

@john2x
Copy link
Owner

john2x commented Jul 17, 2022

Thank you for this. I wasn't aware about the naming requirement for submitting to MELPA, and I do intend to submit this one in the future.

Copy link
Owner

@john2x john2x left a comment

Choose a reason for hiding this comment

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

LGTM

@john2x john2x merged commit 601c1fe into john2x:master Jul 18, 2022
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.

2 participants