Ihor Radchenko <yanta...@posteo.net> writes:
> Mehmet Tekman <mtekma...@gmail.com> writes: >> >> One issue that I cannot seem to solve by myself is getting the >> `org-babel-tangle-sync-mode' to persist on the `after-save-hook' >> after it's been activated. My understanding of this hook is >> that it is global and persists across buffers, but I'm seeing >> some inconsistent behaviour that requires me to toggle the mode >> on and off again. > > Any hook can be local. > See LOCAL argument in help:add-hook Ah, that solves the problem immediately thank you! > Mehmet Tekman <mtekma...@gmail.com> writes: > >> I've modified the ob-tangle.el file for the main tangling and >> detangling functions. Most importantly, both functions can now >> exchange information from the source Org mode file to the target >> remote tangle file in either direction, depending on whether the >> source Org file has `:tangle-sync <action>' in the header. > > Thanks! > >> The action is one of: >> >> - "export" = always transmit information from the source Org mode >> block to the target remote file. >> - "import" = always transmit information from the target remote >> file to the source Org mode block. >> - "skip" = skip the block. >> - "both" = transmit information from source block to target block >> or target block to source, depending on whether the >> tangle or detangle is called from the source buffer or >> the target buffer respectively. > > May it be better to make :tangle header argument compound? > Like ":tangle "file" export". Similar to :results header argument. See > "16.6 Results of Evaluation" section of Org manual. > That's a great idea I had not considered, and would definitely reduce the header bloat, especially since `:tangle-sync' *requires* the `:tangle' header to be there. > Also, some general comments on the patches: > Great! > 2. When naming private functions, "--" should be after library prefix: > org-babel--... > Thanks, I will rename `org-babel-detangle-single-block--from-source'. > 3. Please do not use private functions from third-party libraries. I am > talking about `cl--set-buffer-substring' in particular. > So initially I used `(setf (buffer-substring X Y) new-content)` but I recieved a warning from Emacs that it was an obsolete generalized variable. After some searching I found this entry in an emacs fork used the cl library: https://github.com/emacs-citar/citar/commit/809953a2191d0e3217ffbed9270be9b3cd6abfd2 Since `(require 'cl-lib)' is already imported in ~ob-tangle.el~, I did not think it was too taboo to use. How does one then set the buffer substring? > 4. Please make sure that docstrings clearly describe what the function > does, each of its arguments, and the expected return value. > In the patch, `org-babel-detangle--block-contents', NEAREST is > ambiguous. The actual meaning is "block at point or previous block". > > 5. Pay attention to buffer boundaries. In particular, remember that > buffer may be narrowed when you call a command and that expressions > like (+ 2 (line-beginning-position)) may resolve beyond > `point-min'/`point-max'. > > 6. Avoid using cryptic list functions like `cdadar' when you can use > something more readable. > > 7. When naming functions or macros, make sure that the name is roughly > describing the purpose of the function. In `org-babel-detangle', you > added `single-block-metrics' local function that does not only do the > metrics, but also (unexpectedly!) calls > `org-babel-detangle-single-block'. This is especially important for > local functions that lack docstring. > > 8. In `org-babel-tangle', (setq block-counter (+ 1 block-counter)) > appears to be misplaced into outer sexp level after your patch. Thanks for these, I will clean up and better document my code. > 1. Please make sure that patches do not leave Org in broken state in the > middle of the patchset. Your patch #1 adds two `defun's in the middle > of `org-babel-detangle', which is not right. > > 9. In commit messages, please mark newly added functions clearly. > Like "(org-babel-foo): New function. It does this and that." > Same for commit summaries - "lisp/ob-tangle.el: Detangle a single > block" is awkward. You should clearly indicate that you added > something new to the library. Apologies. I rebased and squashed all my commits into one, and then selectively staged hunks into seperate commits for the git format-patc process. For some reason the diff function decided that the new functions should exist right in the middle of an existing function and I was not sure how to resolve it at the time (though I have a better idea now). I will take better care with the messages. I tried to look for previous "[ANN]" postings in the mailing list that I could emulate, but didn't pay enough attention it seems. I'm finally using `gnus' as my mail client so I'm slowly getting into a more streamlined mindset that should be better at submitting and formatting patches. (To reply to a mailing list, I do a wide reply to the author and hope that the `Mail-Followup-To' header is used?) Apropos patches: Given how broken my current patches are, my next set of changes will be not contingent on the previous ones. I will start a new set of patches. I hope that's okay. Best, Mehmet
