-
Notifications
You must be signed in to change notification settings - Fork 151
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
add org src block feature #411
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not much to say about the actual feature (I'm assuming it does what it should), so I'll just leave some style related comments.
dumb-jump.el
Outdated
:action '(("Jump to match" . dumb-jump-result-follow)) | ||
:candidates (-zip choices results) | ||
:persistent-action 'dumb-jump-helm-persist-action) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem to only be whitespace changes?
dumb-jump.el
Outdated
(let ((alreadyuptodate | ||
(--filter (string= newlang (plist-get it :language)) | ||
proplist))) | ||
(if alreadyuptodate | ||
nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can drop the let here and use an unless instead of if.
dumb-jump.el
Outdated
(if newfindrule | ||
(set-default 'dumb-jump-find-rules newfindrule)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A when is preferable to a single if used for side effects.
;; add (if needed) composite language to dumb-jump-language-file-exts | ||
(if (not dumb-jump-search-type-org-only-org) | ||
(if newfileexts | ||
(set-default 'dumb-jump-language-file-exts newfileexts))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, just with unless.
dumb-jump.el
Outdated
emacs-lisp "elisp" elisp "elisp" | ||
R "r" r "r")) | ||
(result (plist-get lookup (intern lang)))) | ||
(if result result nil))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The if is redundant, but I'd also change the entire function to use assoc/alist-get or cond, instead of plist-get.
(alist-get lang '(("sh" . "shell") ("shell" . "shell") ...))
dumb-jump.el
Outdated
"Run dumb-jump-fetch-results if searcher installed, buffer is saved, | ||
and there's a symbol under point." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first line of a docstring should be a complete sentence (checkdoc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually missed this in a few of the last docstring, that should be fixed too.
dumb-jump.el
Outdated
"In org mode, if inside a src block return | ||
associated language or org when outside a src block." | ||
(let ((lang (nth 0 (org-babel-get-src-block-info)))) | ||
; if lang exists then create a composite language |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a ;; comment here, the indentation if unnecessary.
I don't think that fixing "unrelated" checkdoc issues should be part of this pull request. |
I have undone all "unrelated checkdoc issues" on my local repos. I can push it if needed. |
That would be nice.
…On 29 January 2022 15:27:31 CET, Pierre-Andre ***@***.***> wrote:
I have undone all "unrelated checkdoc issues" on my local repos. I can push it if needed.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
Looks like the 24.x tests are failing with
|
Using (org-)internal variables is bound to trigger issues eventually, but I don't know enough about Org internals to understand what they are being used for. Is there any alternative to using these variables directly? Having dumb-jump depend on a newer version of org would be a sad solution. |
I will investigate on a better solution. I apologize for this proposal that needs to be fine-tuned.
5 févr. 2022 22:55:20 Philip ***@***.***>:
… Using (org-)internal variables is bound to trigger issues eventually, but I don't know enough about Org internals to understand what they are being used for. Is there any alternative to using these variables directly? Having dumb-jump depend on a newer version of org would be a sad solution.
—
Reply to this email directly, view it on GitHub[#411 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AFOUQVFH3HWIE4BR5IN6G6TUZWMEPANCNFSM5MH5KNXQ].
Triage notifications on the go with GitHub Mobile for iOS[https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675] or Android[https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub].
You are receiving this because you authored the thread. [data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEIAAABCCAYAAADjVADoAAAAAXNSR0IArs4c6QAAAARzQklUCAgICHwIZIgAAAAnSURBVHic7cEBDQAAAMKg909tDwcUAAAAAAAAAAAAAAAAAAAAAPBjRFIAASHKmKkAAAAASUVORK5CYII=###24x24:true###][Image de pistage][https://github.com/notifications/beacon/AFOUQVCMNO3GNOGTVQLVFODUZWMEPA5CNFSM5MH5KNX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHVXVDHY.gif]
|
The 24.x tests hung. Retriggering them |
Well I need to set up a docker image of emacs 24.5 and 24.4 to understand what's going on. It will take much more time… |
emacs 24.x tests hung because
I hope that all will be fixed but as docker tests with 24.5 does not work at all I cannot be sure… |
Using the following Dockerfile
I can run the snapshot version and the
(NB: EVM emacs-git-snapshot recipe does not work on my laptop.) Conclusion: these check problems on "snapshot" are not coming from org-mode |
Any updates? |
This PR aims at providing a dumb-jump "definition search" in org-mode src-block. It closes (in some way) issue #135.