search

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

[WIP]Support more :lines formats and internal links #190

Open llcc opened 1 year ago

llcc commented 1 year ago

This PR adds support for a more flexible :lines format and org-mode internal links.

I hope this is helpful! Let me know if you have any other questions.

nobiot commented 1 year ago

@llcc Thank you for the initiative, and I really appreciate the effort.

The intention is great. Let me share two things and make some requests in this context:

  1. There is a branch (feature/org-internal-link) that explores support internal links more generically

    coderef, custom-id, fuzzy, and radio are internal link types. Test radio and fuzzy so far.

    I have encountered some issues in the live-sync feature. Have you encountered anything like it? (the location of the region gets dislocated by 1 point). I'd love to see this tested more thoroughly and any issue, if any reproducible, resolved.

  2. There is another branch (PR #187) trying to refactor function org-transclusion-content-range-of-lines to separate the logic for thing-at-point and lines

    I feel that function org-transclusion-content-range-of-lines has become a bit too complex for me and us (wider community) to continue to maintain it easily for long-term.

    Your PR touches it (right?) too. I would like to get this refactorinng done first before moving on to enhanching it any further. @devcarbon-com has kindly agreed to work on it when he has a bit more time in his hand. I'd appreciate if you could also look at it if you feel up for it.

That's the current context and requests on my end.

For the PR, I feel that you have very personal, concrete use case -- how do you use the current src-lines features and what motivate you to enhance it?

To me :lines -5,7-8,4,9- looks really complex. Appearance of complexity is not an issue but I am sure there is a motivation and concrete issue that you would like to resolve -- would you be able to share your background a bit more by any chance?

nobiot commented 1 year ago

One more ting... Org-transclusion is part of ELPA, this means there is paperwork involved for me to be allowed to accept any contribution. Below is an excerpt of the relevant part from README. Would you be willing to start the process? I don't need you to have completed the process to accept your PR; but the paperwork is required if PRs of more than 15 lines of code to be accepted into the codebase.

Org-transclusion is part of GNU ELPA and thus copyrighted by the Free Software Foundation (FSF). This means that anyone who is making a substantive code contribution will need to “assign the copyright for your contributions to the FSF so that they can be included in GNU Emacs” (Org Mode website).

llcc commented 1 year ago
  1. There is a branch (feature/org-internal-link) that explores support internal links more generically

Okay.. I was previously unaware of the feature/org-internal-link branch. It appears to support more link types than my PR 👍 . Hmm.. But I was unable to get it work with #+transclude: [[table-name]] for #+NAME: table-name. Is this not yet supported? The error is caused by https://github.com/nobiot/org-transclusion/blob/447a3f42eb1afaa3d362bcec479b27493faa339f/org-transclusion-src-lines.el#L132

I have encountered some issues in the live-sync feature. Have you encountered anything like it? (the location of the region gets dislocated by 1 point). I'd love to see this tested more thoroughly and any issue, if any reproducible, resolved.

Although I have not yet had the opportunity to test it, I personally rarely have the need to live edit in the same buffer. I'll take a look on it later.

  1. There is another branch (PR Refactor/src lines #187) trying to refactor function org-transclusion-content-range-of-lines to separate the logic for thing-at-point and lines Your PR touches it (right?) too. I would like to get this refactorinng done first before moving on to enhanching it any further.

Agree on this. I'll wait util your refactor is complete.

For the PR, I feel that you have very personal, concrete use case -- how do you use the current src-lines features and what motivate you to enhance it?

I frequently need to transclude parts of tables or lists into other buffers. For tables, I usually need the header, a horizontal line, and some rows of the table, as well as the last #+tblfm: line. For lists, I usually insert a <<target>> in the parent list, then transclude random one of its sub-lists many times. In these cases, the current :lines range is not enough.

In my PR, I have addressed the limitation by extending the :lines range to consider the bounds of the elements. For example, :lines 1- now transcludes all the way to the table end, instead of to the end of the source buffer. This change makes it easier to transclude parts of tables and lists, or other types of elements.

llcc commented 1 year ago

One more ting... Org-transclusion is part of ELPA, this means there is paperwork involved for me to be allowed to accept any contribution.

I have signed the document. I can send you a copy if you need.

nobiot commented 1 year ago

Thank you for your detail. No need to send me a copy of the FSF paperwork; your statement is enough for me.

I frequently need to transclude parts of tables or lists into other buffers. For tables, I usually need the header, a horizontal line, and some rows of the table, as well as the last #+tblfm: line. For lists, I usually insert a <<target>> in the parent list, then transclude random one of its sub-lists many times. In these cases, the current :lines range is not enough.

From this I understand that this PR is support tables in Org file. Is this correct? Then we should take a different approach. I will follow up on this over the weekend or next week.

Very briefly:

I will come back with more concrete details.

Edit: I only did a quick experiment for tables only but the same principle should apply to lists (to be verified).