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

Generalize counsel-outline to handle major modes and display styles - cont'd #1684

Closed
wants to merge 8 commits into from

Conversation

ericdanan
Copy link
Contributor

@ericdanan ericdanan commented Jul 26, 2018

Re #1681

counsel.el Outdated
adequate or optimal, these variables can be rebound locally in
the major mode hook. Replacements are provided for some such
modes. To set them up, add this to your init file:"
This command's behavior can be customized based on the current buffer's major mode. See variable `counsel-outline-settings'."
Copy link
Collaborator

@basil-conto basil-conto Jul 26, 2018

Choose a reason for hiding this comment

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

Needs more filling! :)

Copy link
Contributor Author

@ericdanan ericdanan Jul 28, 2018

Choose a reason for hiding this comment

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

Not sure what you mean. I agree the sentence is not very informative, but on the other hand I didn't want to duplicate the information of counsel-outline-settings's docstring.

So I just removed the sentence. This actually seems in line with many counsel commands that have short docstrings not mentioning the corresponding custom variables.

But of course if you have suggestions, @abo-abo will make the decision.

counsel.el Outdated
;; `\end{document}', in which case we simply return that.
(if (and (assoc (match-string 1) (if (boundp 'LaTeX-section-list)
LaTeX-section-list
latex-section-alist)) ; section macro
Copy link
Collaborator

@basil-conto basil-conto Jul 26, 2018

Choose a reason for hiding this comment

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

How about:

(or (bound-and-true-p LaTeX-section-list)
    (bound-and-true-p latex-section-alist)))

This removes the need for (defvar LaTeX-section-list) or (defvar latex-section-alist) and makes the function a tiny bit less mode/package-dependent (not that this is particularly important).

Copy link
Contributor Author

@ericdanan ericdanan Jul 28, 2018

Choose a reason for hiding this comment

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

Thanks, done

@ericdanan
Copy link
Contributor Author

@ericdanan ericdanan commented Jul 28, 2018

In the last batch of 4 commits, the first and fourth follow @basil-conto 's comments.

The second one add :history and :caller to counsel-outline and make them customizable in counsel-outine-settings. This allows in particular to perfectly replicate counsel-org-goto for org-mode buffers, and can be useful more generally I think.

The third one transforms the counsel-org-goto-XXX custom variables into counsel-outline-XXX, makes them customizables in counsel-outline-settings, and makes aliases from the former to the latter. Also counsel-org-goto becomes an alias to counsel-outline, counsel-org-goto--get-headlines to counsel-outline-candidates, and counsel-org-goto--add-face to counsel-outline--add-face. This looks more consistent to me and I think it shouldn't alter anyone's setting, but you may of course think otherwise. Perhaps the alias is not needed for the latter two functions (in which case counsel-outline--add-face could be simplified slightly by making the two new optional args required).

I don't plan to push more commits for now, unless you suggest me to do so.

@abo-abo abo-abo closed this Jul 30, 2018
@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Jul 30, 2018

Thanks.

abo-abo pushed a commit that referenced this issue Jul 30, 2018
@basil-conto
Copy link
Collaborator

@basil-conto basil-conto commented Jul 30, 2018

Not sure what you mean.

Sorry @ericdanan, I didn't see your reply to my comment. I was referring to what Emacs calls filling, by which all I meant was that the docstring line was too long and should be wrapped at ~65 columns, as is customary in Elisp.

@ericdanan
Copy link
Contributor Author

@ericdanan ericdanan commented Jul 31, 2018

If verbatim, the face used in the buffer is applied. For simple
headlines in org-mode buffers, this is usually the same as org
except that it depends on how much of the buffer has been
completely loaded. If your buffer exceeds a certain size,
Copy link
Collaborator

@basil-conto basil-conto Aug 2, 2018

Choose a reason for hiding this comment

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

I know the wording wasn't touched by this PR, but since I'm working on improving these docs, I'd like to clarify: instead of "loaded", should this say "fontified" or similar instead? Because I don't understand how a buffer can be partially loaded.

@ericdanan ericdanan deleted the out branch May 6, 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

3 participants