Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ledger-report: Pass ledger-default-date-format to ledger command #406

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bcc32
Copy link
Collaborator

@bcc32 bcc32 commented Mar 18, 2024

Fix #289

@jdek
Copy link

jdek commented Mar 19, 2024

Related to #297, shouldn't we be setting this in the ledgerrc?

@bcc32
Copy link
Collaborator Author

bcc32 commented Mar 20, 2024

I agree, and it does work if you just put it in .ledgerrc instead, but this custom variable already exists, and I feel we may as well make it do what it is documented to do. In addition, it is at least useful to have such a variable to override the behavior in certain contexts. However, I agree that the variable needs some thinking about. It currently doesn't distinguish between output date format and input date format, which ledger-cli does, and in fact ledger-mode reads --input-date-format from ledgerrc. It's all a bit messy, IMO.

@jdek
Copy link

jdek commented Mar 20, 2024

(defun ledger-format-date (&optional date)
"Format DATE according to the current preferred date format.
Returns the current date if DATE is nil or not supplied."
(format-time-string
(or (cdr (assoc "input-date-format" ledger-environment-alist))
ledger-default-date-format)
date))

I don't like implicitly using --input-date-format for the output --date-format. We need to make some clear separation here, with proper fallback to ledgerrc.

@bcc32
Copy link
Collaborator Author

bcc32 commented Mar 20, 2024

I guess I was a little unclear in my previous comment---ledger-format-date is used when ledger-mode wants to insert a date into a ledger file, so defaulting to the user's preferred input format makes sense (i.e., if they specified one in ledgerrc, ledger-mode should automatically insert dates that way when editing ledger files). The only place it is used for output is in reconciliation buffers (see below) and also in ledger-schedule, which I am unfamiliar with.

--input-date-format from ledgerrc is otherwise not passed to --date-format. It is currently the case, I believe, that (with the exception of the bug this PR is designed to address), --date-format <ledger-default-date-format> is always passed to the ledger cli, which overrides ledgerrc's specification of --date-format if the latter is present.

For the reconciliation buffers, it looks like they do not actually use the ledger-reconcile-default-date-format setting at all right now. I think we should fix that bug (opened #408 to track)

@bcc32 bcc32 force-pushed the report-use-default-date-format branch from e2f75fe to 060624b Compare March 21, 2024 22:02
@bcc32 bcc32 force-pushed the report-use-default-date-format branch from 060624b to ab1a951 Compare April 23, 2024 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ledger-default-date-format setting not respected in reports
2 participants