Skip to content
This repository has been archived by the owner on Apr 17, 2024. It is now read-only.

Add imenu support #87

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

colonelpanic8
Copy link

This is sort of WIP. I think that everything works properly, but the code feels a little messy to me.

@colonelpanic8
Copy link
Author

Closes #65

@pashky
Copy link
Owner

pashky commented Jul 29, 2015

Thanks! I'm not dead, just a bit busy at the moment :) I'll review and merge this as soon as I get to proper computer.

@colonelpanic8
Copy link
Author

@pashky good to heal you are still kicking... thanks

@pashky
Copy link
Owner

pashky commented Aug 17, 2015

Ok, so how does one use it? I've never used imenu myself unfortunately, so not familiar with it. I tried calling M-x imenu and got user-error: This buffer cannot use `imenu-default-create-index-function'

What's the purpose of restclient-goto-current-min and splitting restclient-http-parse-current-and-do into two?

@colonelpanic8
Copy link
Author

you tried calling imenu?

Ok, so how does one use it? I've never used imenu myself unfortunately, so not familiar with it. I tried calling M-x imenu and got user-error: This buffer cannot use `imenu-default-create-index-function'

Yeah, you aren't using the imenu functions from this pull request. I think I forgot the line:

(set (make-local-variable 'imenu-create-index-function) #'restclient-create-imenu-index)

which i have in my local copy. Sorry about that. As I mentioned this was sort of WIP. Its in a good working state now though, so ill fix it an update.

What's the purpose of restclient-goto-current-min

I thought it might be useful. Can remove if you don't want it.

splitting restclient-http-parse-current-and-do into two?

Single responsibility principle thing. Originally I was using restclient-http-parse-current to get imenu titles, but then it became clear that that would be far too slow. Still generally think its a good refactor that could be useful to others that want to extend restclient.

@colonelpanic8
Copy link
Author

Ping?

@pashky
Copy link
Owner

pashky commented Nov 18, 2015

I've reviewed your change.

  1. I get compile time warnings

    In restclient-create-imenu-index:
    restclient.el:397:8:Warning: `beginning-of-buffer' is for interactive use
    only; use `(goto-char (point-min))' instead.
    restclient.el:410:38:Warning: assignment to free variable `marker'
    restclient.el:410:27:Warning: reference to free variable `marker'
    
  2. There is debug (message ...) left in code.

  3. Change to (restclient-jump-prev) is unnecessary as it makes it jump to beginning of current request if cursor is before that point. This shouldn't happen, it should just stay where it is if there's nowhere to jump.

  4. Unused (restclient-goto-current-min) is still there.

  5. And it's still not working for me. I'm getting.

restclient-create-imenu-index: Not enough arguments for format string

if I do M-x imenu on sample httpbin file.

@colonelpanic8
Copy link
Author

I get compile time warnings

Fixed

There is debug (message ...) left in code.

Fixed

Change to (restclient-jump-prev) is unnecessary as it makes it jump to beginning of current request if cursor is before that point. This shouldn't happen, it should just stay where it is if there's nowhere to jump.

This was deliberate -- without this change there isn't consistency about where you jump to when navigating between requests. Seems like its better to always go to the uri line when you do a jump forward or backwards. I changed it back for you, but I don't understand why this behavior is preferred

Unused (restclient-goto-current-min) is still there.
And it's still not working for me. I'm getting.
restclient-create-imenu-index: Not enough arguments for format string
if I do M-x imenu on sample httpbin file.

an httpbin file? this is imenu for restclient. I'm assuming you have restclient mode enabled?

Can you M-x toggle-debug-on-errror and give me a stack trace for this

restclient-gooooot-current-min behavior.
@colonelpanic8
Copy link
Author

ping

@blaenk
Copy link

blaenk commented Jul 20, 2016

This would be awesome to have. I absolutely love imenu (particularly helm-imenu, you should try it!).

@colonelpanic8
Copy link
Author

I'm happy to deal with merge conflicts if @pashky will agree to merge this.

I really think that imenu is a better option than direct helm support

@expez
Copy link

expez commented Dec 15, 2017

@IvanMalison Do you have some code in your .init I can copy to make this work? Or a fork of restclient I can use?

I was about to write this myself, but realized it was more work than I have time for right now and a google led me here ;)

@colonelpanic8
Copy link
Author

colonelpanic8 commented Dec 15, 2017

@expez Just use this pull request? I believe its in my fork...
https://github.com/IvanMalison/restclient.el

@sanel
Copy link

sanel commented Feb 11, 2019

@pashky @IvanMalison not sure what is status of this PR, but this simple line can also provide imenu support:

(require 'imenu)
(setq imenu-generic-expression '((nil "^[A-Z]+\s+.+" 0)))

Nothing fancy, just will display things like GET.http://www.httpbin.org and contains request + url to be jumped to.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants