nobiot / org-transclusion

Emacs package to enable transclusion with Org Mode
https://nobiot.github.io/org-transclusion/
GNU General Public License v3.0
917 stars 44 forks source link

Feature: select end via n things at point. #157

Closed devcarbon-com closed 1 year ago

devcarbon-com commented 1 year ago

Sometimes it's nice to be able to transclude a number of THINGS (ie 'paragraphs or 'sexp) instead of a line count.

Still missing: proper tests and doc.

basic usage:

# one paragraph:
#+transclude: [[../blue.cljd::id:1671783193][[hello-world]]  :src clojure :thing-at-point paragraph

# three sexp:
#+transclude: [[../blue.cljd::id:1671783193][[hello-world]]  :src clojure :thing-at-point sexp :end "3"

:end is optional and will just default to 1 THING if absent.

THING can be any valid thing-at-point symbol.

nobiot commented 1 year ago

Wow. I like the intent. I’ll be on holidays; let me come back in the new year! Thank you

devcarbon-com commented 1 year ago

Haven't had a moment to do it yet, but I want to modify this a little bit. Currently, it just assumes the starting point is at the beginning of the line, which works well for certain things like top level functions and paragraphs. But for sexps in particular, it currently doesn't work to try to link to one that is not at the start of the line.

nobiot commented 1 year ago

Sure; I can wait. I am still traveling… But I am not sure if I can be of any help for the implementation anyway; I would need to rely on you.

devcarbon-com commented 1 year ago

No worries. I have a big project I'm working on that has a 3 month deadline, so I'm trying to find the right balance between working on the project, and working on tooling such as this. So I might be a little slow in getting there, but I do expect to do so at some point. :)

devcarbon-com commented 1 year ago

So this is better, it handles indented sexps okay, but it does lack flexibility for determining where point should start. Currently, it assumes the first non-whitespace char of the current line is the point to start.

So:

(defun foo ()
  (bar (let ((fuzz (buzz))) ;<id:1234567890>
            (baz fuzz)))
#+transclude: [[../baz.el::id:1234567890][[barz-baz-fuzz]]  :src elisp :thing-at-point sexp

Would transclude as:

  (bar (let ((fuzz (buzz))) ;<id:1234567890>
            (baz fuzz)))

So you couldn't transclude just the let without moving it to it's own line. I feel like this is good enough for most use-cases.

While we could try to get fancy and allow options for more precise navigation options, I'm much more inclined to add a separate pull request that simply allows the user to specify a function for determining the range. (I have a specific use-case in mind for Clojure code, which is certainly way to specific for org-transclution as a whole.)

I'm done tinkering with this pull request should you like to merge it in, but bare in mind that I have yet to add documentation and proper tests.

devcarbon-com commented 1 year ago

Something I'm noticing now is that this seems to only intermittently work with live-sync. I'm guessing it's my automatic new line addition that breaks it...

Edit: not the newline. Works the first time. Breaks the second. Seems the difference in overlay length is comparing the size of the transclusion before the live-edit. Digging some more...

Edit: Breaks after first live edit even when no changes are made...

Edit: Oh interesting, it only happens with target style links.

Edit: Seems to be an interference of aggressive-indent.

nobiot commented 1 year ago

@devcarbon-com , is the PR at the stage for testing and merging to main? Or are you still massaging the code?

devcarbon-com commented 1 year ago

@nobiot I'd say it's ready.

It does seem to work in most cases. I've been able to narrow down the cause a bit today, it is some sort of interference of aggressive indent. Disabling it I've been unable to reproduce the bug at all.

It would be nice to try to hunt that down further, but I'm not sure I can invest the effort into that at the moment.

devcarbon-com commented 1 year ago

Something worth noting for testing is that I have not really fleshed out how well this works with other things that could be at-point other than sexp and paragraph.

I've written this feature from the perspective of reverse-literate programming, the main driving factor was to be able to transclude code by a tagged ID, so that it would not be broken by rename refactors.

devcarbon-com commented 1 year ago

thing-at-line would probably convey what this does a little better than thing-at-point perhaps.

ParetoOptimalDev commented 1 year ago

Still missing: proper tests and doc.

Honestly with just minimal docs and happy path tests for most used code paths I'd say this is mergeable.

nobiot commented 1 year ago

I am hoping to spend some focus time on reviewing the PR this weekend (long weekend with labour day where I live). I will need to start getting familiar with the intent of the PR.

@devcarbon-com would you be able to do some paragraphs to describe the following briefly for the feature as it stands now?specifies a "thing" -- w

I can update the user manual (you may if you like -- it should be automatically build when I merge the PR to main).

I am guessing that with this feature, you can specify a "thing" to transclude with:thing-at-point (or :thing-at-line). The intended use case is shown here https://github.com/nobiot/org-transclusion/pull/157#issuecomment-1449162789:

#+transclude: [[../baz.el::id:1234567890][[barz-baz-fuzz]]  :src elisp :thing-at-point sexp
(defun foo ()
  (bar (let ((fuzz (buzz))) ;<id:1234567890>
            (baz fuzz)))

I believe the "::id:123456789" part leverages the built-in Org's search capability (which Org-transclusion makes use of). The translusion starts with the line that has the ID (the first occurrence of the buffer if more than one), and the end point is specified by the :thing-at-point.

As it stands now, you cannot use :thing-at-point to specify the precise beginning of the thing within a line -- it is always the beginning of the line -- I have no problem with this, but is it a limitation (to be hopefully fixed) or ... by design? (feature as is).

I would like to know especially the following:

Thank you!

devcarbon-com commented 1 year ago

@nobiot

What are the allowed (intended?) or tested "things"?

I've briefly tested these:

forward-thing is the function used for selecting multiple things. Some gotchas from some of the less common things could stem from this not handing them as expected.

I expect these will be more common:

I'm curious if defun will work as expected across different languages. Python or C++ for example. I haven't tested that yet.

What "things" are known not to work? (limitations)

So far the only known limitation is the inability to select where it starts. This was added for ease of selecting a line by id instead of contents (rarely if ever would you want the thing to be the id marker).

But this is probably most applicable to a reverse-literate setup, whereas this feature as a whole could have a broader range of uses.

(Like transcluding a paragraph from a text document without having to specify it's precise location and/or length of lines, as an example.)

#+transclude: [[../story.txt::Once upon a time][[story]]  :thing-at-point paragraph

This could be changed to be the point at which the org link takes you, instead of calling back-to-indentation.

So now for reverse-literate purposes, I could add a new pull request for a :selector tag, which could dispatch selecting the transclusion range to any function.

Perhaps something like this: No extra args

#+transclude: [[../baz.el::id:123][[barz]]  :selector my-custom-select-fn

Extra args

#+transclude: [[../baz.el::id:123][[barz]]  :selector (my-custom-select-fn any-extra args)
;; This could be a little misleading, since it looks like a function call. 
;; Maybe use square brackets instead, or just quote the list.

Then the user/emacs-package could define their own means of navigating and selecting the range with tree-sitter, or calling org navigation functions, or whatnot.

(defun my-custom-select-fn ()
  (magical-mark-what-i-mean)
  (list (region-beginning) (region-end)))
nobiot commented 1 year ago

Added a dev/feature branch https://github.com/nobiot/org-transclusion/tree/dev/feature--things-at-point.

Added some test files

Test the following: sentence paragraph defun sexp list

They seem to work with three (minor) observations. @devcarbon-com , do you have some idea how we might frame the issues? (ignore or some way to "fix" them?)

[NOTE: Below, I think it's not really easy to see the images and text. Let me quickly post this reply. I think I will come back to edit it for better legibility]

  1. When transcluding sentence, The trailing \n seems to be removed -- in effect, resulting in the brank line removed. See the following for illustration:

image Figure 1. Before tansclusion


image Figure 2. After tansclusion -- note the transcluded sentence is followed immediately by the next transclusion keyword; I believe there should be an additional line break here (?)


  1. How do we test list? I don't know what it is supposed to be.

  2. Transcluding defun with the start point in the middle of a function seems to transclude only sexp not the entire defun.

#+transclude: [[./things-at-point-dir/baz.el::id:1234567890][[barz-baz-fuzz]] :src elisp :thing-at-point defun


image Figure 3. Before tansclusion of defun


image Figure 4. After tansclusion of defun


The rest seems to work -- for defun I have only tested Elisp....

devcarbon-com commented 1 year ago

Ah, good catch @nobiot!

I'll look into that this evening and see if I can pin it down.

reyman commented 1 year ago

This is just a comment for later development, but, with the advent of tree-sitter and possibility of structural parsing, i suppose the future of "selecting the exact part of code source" to transclude will be more easy ?

devcarbon-com commented 1 year ago

@reyman Yes, and I plan to make use of it in the reverse-literate package I'm working on that heavily leans on org-transclusion.

I haven't added it yet, but I plan to add a PR that would allow ANY means of selecting the range.

#+transclude: [[../baz.el::id:123][[barz]]  :selector my-custom-select-fn
nobiot commented 1 year ago

It will be great.

As for this current PR, I am trying to refactor it a little to contain all the intended changes within lines-src.el file…

devcarbon-com commented 1 year ago

Thanks @nobiot, I've been without a lot of spare time, so I'm especially glad for your help in lightening the load :slightly_smiling_face:.

nobiot commented 1 year ago

I have merged this branch and refactored. In order to show history correctly, I am going to merge this PR, and then push my overriding main branch. Hopefully it will all work.

nobiot commented 1 year ago

User manual mentions the :thing-at-point property. https://nobiot.github.io/org-transclusion/#thing_002dat_002dpoint-property-to-specify-a-_0060_0060thing_0027_0027-to-transclude-from-the-source. This appears in the feature index too (screen below)

Also added a comment in the "Known Limitation" section.

With the user manual, I have a feeling that I should re-organize a bit to make these properties appear more prominently for easier look up from the TOC, etc... Any idea would be welcome.

image

llcc commented 1 year ago

Hi, guys! Thanks for making this PR into Org-transclusion.

I want to suggest using the single-word thingatpt instead of thing-at-point, as thingatpt is more concise. BTW, the name of Emacs thing-at-point library is thingatpt.

What are your opinions?

nobiot commented 1 year ago

Good idea but we should keep supporting thing-at-point for backward compatibility now that it's been published to GNU-devel repository.

The minimal change like the diff below lets users use :thingatpt as an acceptable alternative property name in the #+transclude keyword like this: ":thingatpt sexp" (full line example below).

#+transclude: [[./things-at-point-dir/baz.el::foo][barz-baz-fuzz]]  :src elisp :thingatpt sexp

When you add and then remove the transclusion, it will automatically go back to the canonical property name :thing-at-point. Perhaps we could start with this?

Retaining the original :thingatpt as users put is possible but it requires more changes and at least I don't have capacity to do this (with properly testing it)). PR is welcome if someone is willing to help us for this.

Thoughts?

@@ -311,8 +311,8 @@ It is meant to be used by `org-transclusion-get-string-to-plist'.
 It needs to be set in `org-transclusion-get-keyword-values-hook'.
 Double qutations are optional :thing-at-point \"sexp\".  The regex should
 match any valid elisp symbol (but please don't quote it)."
-  (when (string-match ":thing-at-point \\([[:alnum:][:punct:]]+\\)" string)
-    (list :thing-at-point (org-strip-quotes (match-string 1 string)))))
+  (when (string-match "\\(:thing-at-point\\|:thingatpt\\) \\([[:alnum:][:punct:]]+\\)" string)
+    (list :thing-at-point (org-strip-quotes (match-string 2 string)))))

 (defun org-transclusion-content-format-src-lines (type content indent)
   "Format text CONTENT from source before transcluding.
nobiot commented 1 year ago

@llcc It was possible quicker than I thought it would be. Created a PR #189 to:

  1. Allow :thingatpt as an alternative property name
  2. Retain the original as it is typed by the user

@llcc, could you, or someone test the branch, please?

Here is the file I used for my quick manual testing.

ParetoOptimalDev commented 10 months ago

the reverse-literate package I'm working on that heavily leans on org-transclusion.

@devcarbon-com Is there a repo online anywhere so I can try this or at least follow your progress on this?

devcarbon-com commented 10 months ago

@ParetoOptimalDev Sure, it is in a proof-of-concept stage at the moment, and I've not been able to work on it recently (hopefully soon).

https://github.com/devcarbon-com/jorana.el/