Re: [O] contrib/lisp/ox-taskjuggler.el patch

[Nicolas Goaziou]

Hello,

Глеб Бородуля <address@hidden> 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