Re: contrib/lisp/ox-taskjuggler.el patch

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Глеб Бородуля <mr.reg@mail.ru> writes:

> I want to pull some changes in contrib/lisp/ox-taskjuggler.el file
> There is patch in attach, created with git format-patch
> Manually written change log here:

Thank you for the patch. Comments follow.

> ox-taskjuggler.el: Minor enhancements in taskjuggler export

It would be better to split these enhancements into as many patches.

> * contrib/lisp/ox-taskjuggler.el b/contrib/lisp/ox-taskjuggler.el
> (org-taskjuggler--build-project): added new custom variable
> `org-taskjuggler-default-project-configuration' that can be used to
> configure additional project fields.

You need to capitalize the sentence. Also, it looks like you're using an
outdated "ox-taskjuggler.el". See below.

> (org-taskjuggler-get-end): added support for `end' property instead
> only standard org deadline value.

Ditto.

> (org-taskjuggler--build-task): added support for multiple allocations,
> specified by separators.

Not sure about the usefulness of this one. According to
<http://www.taskjuggler.org/tj3/manual/allocate.html>,

  allocate task1, task2, task3

is valid. There's apparently no need to split the task list.

> -    (and deadline (org-timestamp-format deadline "%Y-%02m-%02d"))))
> +    (or
> +     (and deadline (org-timestamp-format deadline "%Y-%02m-%02d"))
> +     (and (memq 'end org-taskjuggler-valid-task-attributes)
> +	  (org-element-property :END item)))))

You don't need to check `org-taskjuggler-valid-task-attributes' here
since you're not applying it to a task.

>  channel.  If no start date is specified, start today.  If no end
>  date is specified, end `org-taskjuggler-default-project-duration'
>  days from now."
> -  (format "project %s \"%s\" \"%s\" %s %s {\n}\n"
> +  (format "project %s \"%s\" \"%s\" %s %s {%s\n}\n"
>            (org-taskjuggler-get-id project info)
>            (org-taskjuggler-get-name project)
>            ;; Version is obtained through :TASKJUGGLER_VERSION:
> @@ -726,7 +734,10 @@ days from now."
>                (format-time-string "%Y-%m-%d"))
>            (let ((end (org-taskjuggler-get-end project)))
>              (or (and end (format "- %s" end))
> -                (format "+%sd" org-taskjuggler-default-project-duration)))))
> +                (format "+%sd" org-taskjuggler-default-project-duration)))
> +	  (mapconcat
> +	   'org-element-normalize-string
> +	   org-taskjuggler-default-project-configuration "")))

Current definition for org-taskjuggler--build-project is

  (concat
     ;; Opening project.
     (format "project %s \"%s\" \"%s\" %s %s {\n"
  	   (org-taskjuggler-get-id project info)
  	   (org-taskjuggler-get-name project)
  	   ;; Version is obtained through :TASKJUGGLER_VERSION:
  	   ;; property or `org-taskjuggler-default-project-version'.
  	   (or (org-element-property :VERSION project)
  	       org-taskjuggler-default-project-version)
  	   (or (org-taskjuggler-get-start project)
  	       (format-time-string "%Y-%m-%d"))
  	   (let ((end (org-taskjuggler-get-end project)))
  	     (or (and end (format "- %s" end))
  		 (format "+%sd"
  			 org-taskjuggler-default-project-duration))))
     ;; Add attributes.
     (org-taskjuggler--indent-string
      (org-taskjuggler--build-attributes
       project org-taskjuggler-valid-project-attributes))
     ;; Closing project.
     "}\n")

Is `org-taskjuggler-default-project-configuration' needed considering
the attributes part above?

> -          (format "  purge %s\n  allocate %s\n"
> -                  ;; Compatibility for previous TaskJuggler versions.
> -                  (if (>= org-taskjuggler-target-version 3.0) "allocate"
> -                    "allocations")
> -                  allocate))
> +       (concat (format "  purge %s\n"
> +		       ;; Compatibility for previous TaskJuggler versions.
> +		       (if (>= org-taskjuggler-target-version 3.0) "allocate"
> +			 "allocations"))
> +	       ;;support for several allocations divided by separator
> +	       (mapconcat #'(lambda (allocation)
> +			      (format "  allocate %s\n" allocation))
> +			  (org-split-string allocate "[ ,]* +")
> +			  "")))

Is it really useful? Also, there's no need for #' in front of a lambda.


Regards,

-- 
Nicolas Goaziou