Ihor Radchenko <[email protected]> writes:

> Björn Kettunen <[email protected]> writes:
>
>>>> You can first form a full list of files via org-add-archive-files
>>>> (if
>>>> needed) in org-dblock-write:clocktable.
>>
>> So basically adding the archive files twice, once in
>> org-dblock-write:clocktable and then in
>> org-clock-get-table-data-with-archives?
>
> Nope. You can change org-clock-get-table-data-with-archives to just
> accept a list of files to merge (archives or not). Then you can call
> org-add-archive-files once in org-dblock-write:clocktable and the
> reuse
> the value.

Is calling org-add-archive-files twice better then calling
org-agenda-prepare-buffers multiple times instead of once for all files?

>> Regarding the when-let in org-clock-merge-table-data. Does it make a
>> difference performance wise to append and entry which might be nil
>> vs
>> appending only if a new entry would not be nil?
>
> I do not think that there will be much difference here.
>
>> Below the updated patch, plus the other change to make the git hook
>> install work if org mode is a git submodule working.
>
> Thanks!
> Comments inline.
>
>> +*** New function ~org-clock-merge-table-data~
>> +
>> +Takes a list of clocktable data tables, merge them all
>> +under file or the file name of the first table.
>
> I think that this new function is not important enough for all Org
> users
> to care. No need to put it into the news.
> Agenda one is more important - I have seen a number of questions that
> would require something similar.

Yeah I noticed the same in my personal setup when it came to setting up
org-caldav with a more complex setup of files.

>> +(defun org-clock--get-table-data1 (file params)
>> +  "Get clocktable-data for FILE with PARAMS."
>
> This docstring is a bit confusing. Maybe you can provide more details
> about what the function does.

Is it about the clock table data being confusing?

Is this better?
"Visit FILE and get clocktable-data with PARAMS."

>> Subject: [PATCH 2/2] * mk/targets.mk (GIT_HOOKS): Resolve correctly
>> when in
>>  submodule
>> ...
>> -GITDIR        = .git/hooks
>> -GITHOOKS      = $(notdir $(wildcard git-hooks/*))
>> +GITDIR        = $(shell git rev-parse --git-path hooks)
>> +GITHOOKS = $(notdir $(wildcard $(GITDIR)/git-hooks/*))
>
> You set GITHOOKS to .git/hooks/git-hooks/*, while it should simply be
> $ROOT/git-hooks/*
ROOT being the repository root? I changed it this way now:
@@ -9,8 +9,9 @@ SUBDIRS       = $(OTHERDIRS) $(LISPDIRS)
 INSTSUB       = $(SUBDIRS:%=install-%)
 ORG_MAKE_DOC ?= info html pdf
 
-GITDIR        = $(shell git rev-parse --git-path hooks)
-GITHOOKS      = $(notdir $(wildcard $(GITDIR)/git-hooks/*))
+GITHOOKDIR     = $(shell git rev-parse --git-path hooks)
+GITTOPLEVELDIR = $(shell git rev-parse --show-toplevel)
+GITHOOKS       = $(notdir $(wildcard $(GITTOPLEVELDIR)/git-hooks/*))
 
 ifneq ($(wildcard .git),)
   # Use the org.el header.
@@ -131,19 +132,19 @@ autoloads: lisp
        $(MAKE) -C $< $@
 
 repro: cleanall autoloads
-       -@$(REPRO) &
+       $(REPRO)
 
-$(GITDIR):
+$(GITHOOKDIR):
        mkdir -p "$@"
 
 # Implicit rule to copy Git hooks in
-$(GITDIR)/%: git-hooks/% | $(GITDIR)
+$(GITHOOKDIR)/%: git-hooks/% | $(GITHOOKDIR)
        cp -f $< $@
 
-githooks: $(addprefix $(GITDIR)/,$(GITHOOKS))
+githooks: $(addprefix $(GITHOOKDIR)/,$(GITHOOKS))
 
 cleangithooks:
-       $(RM) $(addprefix $(GITDIR)/,$(GITHOOKS))
+       $(RM) $(addprefix $(GITHOOKDIR)/,$(GITHOOKS))
 
 cleandirs:
        $(foreach dir, $(SUBDIRS), $(MAKE) -C $(dir) cleanall;)


I will send the updated patch together with other changes.

Reply via email to