add org src block feature #411

Pierre-Andre · 2022-01-18T18:42:16Z

This PR aims at providing a dumb-jump "definition search" in org-mode src-block. It closes (in some way) issue #135.

phikal

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.

phikal · 2022-01-23T11:45:33Z

dumb-jump.el

+                              :action '(("Jump to match" . dumb-jump-result-follow))
+                              :candidates (-zip choices results)
+                              :persistent-action 'dumb-jump-helm-persist-action)


These seem to only be whitespace changes?

phikal · 2022-01-23T11:47:07Z

dumb-jump.el

+  (let ((alreadyuptodate
+         (--filter (string= newlang (plist-get it :language))
+                   proplist)))
+    (if alreadyuptodate
+        nil


You can drop the let here and use an unless instead of if.

phikal · 2022-01-23T11:48:01Z

dumb-jump.el

+    (if newfindrule
+        (set-default 'dumb-jump-find-rules newfindrule))


A when is preferable to a single if used for side effects.

phikal · 2022-01-23T11:48:36Z

dumb-jump.el

+    ;; 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)))


Same as above, just with unless.

phikal · 2022-01-23T11:52:25Z

dumb-jump.el

+                      emacs-lisp "elisp" elisp "elisp"
+                      R "r" r "r"))
+         (result (plist-get lookup (intern lang))))
+    (if result result nil)))


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") ...))

phikal · 2022-01-23T11:54:20Z

dumb-jump.el

+  "Run dumb-jump-fetch-results if searcher installed, buffer is saved,
+and there's a symbol under point."


The first line of a docstring should be a complete sentence (checkdoc).

I actually missed this in a few of the last docstring, that should be fixed too.

phikal · 2022-01-23T11:56:50Z

dumb-jump.el

+  "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


Use a ;; comment here, the indentation if unnecessary.

phikal · 2022-01-29T10:54:05Z

I don't think that fixing "unrelated" checkdoc issues should be part of this pull request.

Pierre-Andre · 2022-01-29T14:27:21Z

I have undone all "unrelated checkdoc issues" on my local repos. I can push it if needed.

phikal · 2022-01-29T15:36:50Z

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.

jacktasia · 2022-02-05T19:59:46Z

Looks like the 24.x tests are failing with

Symbol's value as variable is void: org-src--beg-marker

phikal · 2022-02-05T21:55:08Z

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.

Pierre-Andre · 2022-02-06T21:05:56Z

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]

jacktasia · 2022-02-08T05:40:32Z

The 24.x tests hung. Retriggering them

Pierre-Andre · 2022-02-08T09:14:50Z

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…

Pierre-Andre · 2022-03-04T17:23:06Z

emacs 24.x tests hung because

silversearch-ag version is too old (2.1.0 needed)
-> fix: test are done with rg using dumb-jump-force-searcher
org-version is less than 9 and it implies that
- there is a bug in org-edit-src-exit : when a src-edit buffer is openned with org-edit-src-code and no change is made and the src-buffer is closed with org-edit-src-exit the org buffer is seen (wrongly) as modified -> fix: I added (save-buffer) for these versions
- there another bug in dumb-jump determination of project-root and the test cannot be done properly -> fix: I set dumb-jump-project

I hope that all will be fixed but as docker tests with 24.5 does not work at all I cannot be sure…

Pierre-Andre · 2022-03-25T11:25:29Z

Using the following Dockerfile

FROM debian
## basic downloads clients
RUN apt-get update && \
    apt-get --no-install-recommends install -y \
         ca-certificates \
         curl \
         git \
         openssh-client \
    && rm -rf /var/lib/apt/lists/*

## for emacs building process
RUN apt-get update && \
    apt-get --no-install-recommends install -y \
        autoconf \
        autogen \
        gcc \
        libgnutls28-dev \
        libncurses-dev \
        make \
        pkg-config \
        texinfo \
    && rm -rf /var/lib/apt/lists/*



######## silvergrep
RUN apt-get update && \
    apt-get --no-install-recommends install -y \
        silversearcher-ag  \
    && rm -rf /var/lib/apt/lists/*

######## ripgrep
RUN curl -LO https://github.com/BurntSushi/ripgrep/releases/download/0.10.0/ripgrep_0.10.0_amd64.deb && dpkg -i ripgrep_0.10.0_amd64.deb && rm ripgrep_0.10.0_amd64.deb && rm -f /var/cache/apt/archives/*.deb /var/cache/apt/archives/partial/*.deb /var/cache/apt/*.bin || true

######## Emacs
RUN cd / && mkdir -p /src && cd /src \
  && git clone --depth 1 git://git.sv.gnu.org/emacs.git emacs
 RUN cd /src/emacs   \
     && ./autogen.sh \
     && ./configure  --without-gif\
                          --with-x-toolkit=no \
                          --without-x \
                          --without-all \
                          --with-gnutls \
                          --prefix="/usr/local" \
     && make  && make install
# #
# ######### Cask
ENV PATH="/root/.cask/bin:$PATH"
RUN mkdir -p /CASK && cd /CASK \
     && git clone https://github.com/cask/cask /CASK/.cask
ENV PATH="/CASK/.cask/bin:$PATH"

######### misc
RUN mkdir -p DUMBJUMP
WORKDIR /DUMBJUMP


ENTRYPOINT ["bash"]

I can run the snapshot version and the

dumb-jump-goto-file-line-test failed (the (mock (ring-insert * *)) throws an error and does not seem to be useful here)
other tests passed

(NB: EVM emacs-git-snapshot recipe does not work on my laptop.)

Conclusion: these check problems on "snapshot" are not coming from org-mode

add org src block feature

3ba4a28

jacktasia requested a review from phikal January 18, 2022 18:55

phikal suggested changes Jan 23, 2022

View reviewed changes

Org: include comments from Phikal

eb08812

Org: undo all checkdoc issues, org-mode src block only

7cb85a8

phikal previously approved these changes Jan 30, 2022

View reviewed changes

Org: remove call to org-internal function

3e09990

Pierre-Andre dismissed phikal’s stale review via 3e09990 February 7, 2022 16:06

new tests to cope with old versions of AG and orgmode

1866752

Pierre-Andre added 2 commits December 4, 2022 15:56

Merge branch 'master' into orgmode-src-block

5089873

Merge branch 'jacktasia:master' into orgmode-src-block

bad69c4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add org src block feature #411

add org src block feature #411

Pierre-Andre commented Jan 18, 2022

phikal left a comment

phikal Jan 23, 2022

phikal Jan 23, 2022

phikal Jan 23, 2022

phikal Jan 23, 2022

phikal Jan 23, 2022

phikal Jan 23, 2022

phikal Jan 23, 2022

phikal Jan 23, 2022

phikal commented Jan 29, 2022

Pierre-Andre commented Jan 29, 2022

phikal commented Jan 29, 2022 via email

jacktasia commented Feb 5, 2022

phikal commented Feb 5, 2022

Pierre-Andre commented Feb 6, 2022 via email

jacktasia commented Feb 8, 2022

Pierre-Andre commented Feb 8, 2022

Pierre-Andre commented Mar 4, 2022

Pierre-Andre commented Mar 25, 2022

		(if newfindrule
		(set-default 'dumb-jump-find-rules newfindrule))

		"Run dumb-jump-fetch-results if searcher installed, buffer is saved,
		and there's a symbol under point."

add org src block feature #411

Are you sure you want to change the base?

add org src block feature #411

Conversation

Pierre-Andre commented Jan 18, 2022

phikal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phikal commented Jan 29, 2022

Pierre-Andre commented Jan 29, 2022

phikal commented Jan 29, 2022 via email

jacktasia commented Feb 5, 2022

phikal commented Feb 5, 2022

Pierre-Andre commented Feb 6, 2022 via email

jacktasia commented Feb 8, 2022

Pierre-Andre commented Feb 8, 2022

Pierre-Andre commented Mar 4, 2022

Pierre-Andre commented Mar 25, 2022