bradyt / cone

A ledger.
https://cone.tangential.info
20 stars 5 forks source link

Use date separator from file #48

Closed bradyt closed 4 years ago

bradyt commented 4 years ago

An IRC user suggests they prefer "/" to "-" as date separator. It might be easy to just parse the date separator from the first transaction in the file, and use that for new transactions.

delexi commented 4 years ago

What about the case where there is no transaction in the file?

bradyt commented 4 years ago

How about "-"?

delexi commented 4 years ago

You mean, just hard code a fall back?

Let me explain where I am coming from, maybe that is a use case worth considering: I use cone as an inbox where I enter transactions while I am on the go. Every one or two weeks I import those transactions into my ledger file, clear the inbox file written to by cone and start over. The first transaction would now use "/" as a separator, as cone is faced with an empty file. For whatever reason (I cannot remember), I use "/" as a date separator. Hard coding a fallback will always choose the wrong separator in my use case. The only solution to my problem would be a setting in cone, as far as I can see.

At the moment, I am just replacing the "-" with "/" in my import script, so I have a working setup. I just wanted to give you one more data point to consider.

bradyt commented 4 years ago

Thank you for expanding on the scenario.

I don't think beancount even allows "/". Hledger has an open issue (933) to move from "/" to "-". I think the reasoning for both is that "-" is suggested by ISO 8601.

What breakage do you see if your import does not convert "-" to "/"?

Should we get date separator from most recently added transaction, instead of the first?

In general, I think a good goal is to reduce and minimize how many settings need to be done in cone's settings page.

Potentially we could have a second auxiliary journal file, that could hold lists of accounts, commodity directives, etc. Maybe we could search that file for date formats, even if it's in comments.

ghost commented 4 years ago

I think this issue spans more than just date formatting, in general, how cone formats entries might be different from how the user does (e.g. it's wildly different for me).

If this were desktop I'd have suggested some script to format the output, but I doubt that's appropriate for Android. Perhaps an alternate solution is one akin to what I've done in some of my python software

A solution like that knocks out two birds in one stone: it consists of a single option and makes for a highly flexible formatter

bradyt commented 4 years ago

@w1d3m0d3 There's an existing issue for indentation of postings and alignment of amounts, at https://github.com/bradyt/cone/issues/33. Are there other differences in how cone formats entries from how the user might? Specifics would make the discussion easier for me.

justinjk007 commented 4 years ago

Mmmm this problem annoys me too much. Can I start working on parsing the file to find the separator and falling back to '-' like @bradyt says lol?

Hi, I was doing some research before making a ledger entry app myself and I stumbled upon, I was surprised again when I saw this was written in flutter. I was surprised again when I saw the guy who wrote the dart server mode wrote this. I've been using the app for couple of weeks now and so far so good.

bradyt commented 4 years ago

Greetings @justinjk007,

Can I start working on parsing the file to find the separator and falling back to '-' like @bradyt says lol?

Yes, that sounds great!

I was surprised again when I saw the guy who wrote the dart server mode wrote this.

Hrm. I wouldn't take much credit. But let's spare other readers from trying to clarify this further, perhaps it is off-topic enough. There are the git histories, if anyone is curious.

I've been using the app for couple of weeks now and so far so good.

Great to hear!

justinjk007 commented 4 years ago

Well, I've been jumping around the source for an hour now, I have came to the conclusion to add a new date format variable in file_model.dart and then fill the format after inspecting the code from the refreshContents function. Will that be alright ?

When I messed with flutter in the past I didn't pay much attention to managing state so having a set of model files to manage state is new to me ? Is this what you call scoped model or is this bloc ? IF I want to look into any of those.

Edit: I've done this checkout my PR #55

bradyt commented 4 years ago

Hmm. Unfortunately I would like to take one step backwards in this conversation. I was being too brief in my original reply.

Why aren't you using '-' for date separator? Are you using an older hledger? At this point, all the popular plaint text ledger apps are preferring '-'.

When I messed with flutter in the past I didn't pay much attention to managing state so having a set of model files to manage state is new to me ? Is this what you call scoped model or is this bloc ? IF I want to look into any of those.

I'm just using Provider. Strictly speaking, I'm not using a state management framework. My interpretation is, we are using Provider to lift the state and logic out of the presentation code, so that it can be organized independently. Beyond that, state and logic are just organized according to my humble intuition.

justinjk007 commented 4 years ago

I use ledger cli, not even hledger. Also the code I wrote checks if the user uses / or then fall backs to - for default. Do we not want that anymore? / is what ledger-cli uses by default.

bradyt commented 4 years ago

Ah, I see, from https://www.ledger-cli.org/3.0/doc/ledger3.html,

The default uses a date like ‘2004/08/01’, which represents the default date format of %Y/%m/%d.

I just filed an issue at ledger, at https://github.com/ledger/ledger/issues/1853. Let's see how that goes.

Note that hledger just merged a pull request in the last two weeks, regarding the date separator, at https://github.com/simonmichael/hledger/pull/1157.

justinjk007 commented 4 years ago

I'm sure many people will have this problem for the time being, why not support both ?

bradyt commented 4 years ago

Unfortunately, with my current schedule, this is not at the top of my priority list, so I can't immediately consider all facets of this.

This might help in the mean time, provided by a user on IRC Freenode's #ledger.

I have this in my .ledgerrc:

; Use ISO 8601 dates and times.
--date-format %Y-%m-%d
--datetime-format %Y-%m-%dT%H:%M:%S
justinjk007 commented 4 years ago

Hmmmm, I honestly don't want to change years worth of ledger files. I will just continue using the modified version of the app for now.

Will you merge if John from ledger-cli ends up keeping the old format as default?

On Sun, Jan 12, 2020, 6:23 PM Brady Trainor notifications@github.com wrote:

Unfortunately, with my current schedule, this is not at the top of my priority list, so I can't immediately consider all facets of this.

This might help in the mean time, provided by a user on IRC Freenode's

ledger.

I have this in my .ledgerrc:

; Use ISO 8601 dates and times. --date-format %Y-%m-%d --datetime-format %Y-%m-%dT%H:%M:%S

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bradyt/cone/issues/48?email_source=notifications&email_token=ADCRQOY6CJGLOFARSIHPCIDQ5OQ6TA5CNFSM4JBZGEZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIXHGFA#issuecomment-573469460, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCRQOYLETALPEDAQQ2ATJTQ5OQ6TANCNFSM4JBZGEZQ .

bradyt commented 4 years ago

I will try to merge this either way.

But let's try to introduce as few features as possible. Potentially, if this is in cone forever, we could take a few steps to avoid breaking someone's workflow by a future change.

Can we limit to beginning of line? In other words, might we replace

r'[0-9]{4}/[0-9]{2}/[0-9]{2}'

with the following?

r'^[0-9]{4}/[0-9]{2}/[0-9]{2}'

I think even r'^[0-9]+/' could suffice. But one might consider that less readable.

justinjk007 commented 4 years ago

Yes, I think so too. I will add that.

On Sun, Jan 12, 2020, 7:14 PM Brady Trainor notifications@github.com wrote:

I will try to merge this either way.

But let's try to introduce as few features as possible. Potentially, if this is in cone forever, we could take a few steps to avoid breaking someone's workflow by a future change.

Can we limit to beginning of line? In other words, might we replace

r'[0-9]{4}/[0-9]{2}/[0-9]{2}'

with the following?

r'^[0-9]{4}/[0-9]{2}/[0-9]{2}'

I think even r'^[0-9]+/' could suffice.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bradyt/cone/issues/48?email_source=notifications&email_token=ADCRQO4DSRJIDZ2ZIKVIYFLQ5OW7RA5CNFSM4JBZGEZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIXILDI#issuecomment-573474189, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCRQO3XDOMXOTXTF3V3X3TQ5OW7RANCNFSM4JBZGEZQ .

bradyt commented 4 years ago

@justinjk007 's solution is available at https://github.com/bradyt/cone/tree/629445b7c3098fb91874b559ebab99408adaf1f9, and on Google Play as version 0.2.19. Hoping to push to F-Droid soon.

As the issue is raised, I think this solution is complete enough. We have omitted the case of other date separator's, such as '.', but I presume this is very rare. I think if someone comments here, we could quickly fix.

As for the other issues, I think they are very good ideas and questions. But I think they can be raised as new issues, for focussed discussion.

In particular, I would like it if we found a neat idea, for how GUI applications can be configured to work with plain text data files. Would we pass cli-like args in an auxiliary file? I would like to minimize the amount of configuration occurring in the cone app,

If you feel this needs to be reopened, let me know. Given the open-ended aspect of this thread, I think it's okay to comment here as well, if you prefer.

Thanks all, for the ideas and solutions!

Anachron commented 4 years ago

We have omitted the case of other date separator's, such as '.', but I presume this is very rare.

I'm from Europe and actually use ., like any other country here. Not sure why you say its rare. https://en.wikipedia.org/wiki/Date_format_by_country

--input-date-format %d.%m.%Y
--date-format %d.%m.%Y
bradyt commented 4 years ago

@Anachron I haven't reviewed the code completely, but I think I've moved that logic to this:

  (List<Transaction> transactions) => (transactions.last.date.contains('/'))
      ? DateFormat('yyyy/MM/dd')
      : DateFormat('yyyy-MM-dd'),

So then we could add one more test and potentially output DateFormat('yyyy.MM.dd')?

Anachron commented 4 years ago

You mean DateFormat('dd.MM.yyyy'), right?

Yeah, sure, go ahead. :)

bradyt commented 4 years ago

You mean DateFormat('dd.MM.yyyy'), right?

No.

If you would like a feature added, can you file a new issue, with a title like, "Write dates as day month year", little-endian dates or whatever?

Yeah, sure, go ahead. :)

I don't know when I'll get around to adding period as date separator, FYI. Pull requests welcome.

bradyt commented 4 years ago

The good news is, I think we could avoid adding a feature to the Settings page. hledger author points out to me, the option can be in file.

> cat blah.dat
; -*- ledger -*-

--date-format %d.%m.%Y
--input-date-format %d.%m.%Y

18.02.2020 hello world
    expenses:blah         30.00 USD
    assets:checking      -30.00 USD
> ledger -f blah.dat print
18.02.2020 hello world
    expenses:blah                          30.00 USD
    assets:checking
bradyt commented 4 years ago

Presumably fixed at https://github.com/bradyt/cone/commit/519be58841a48be3c21a3ac47053a15187702beb.

justinjk007 commented 4 years ago

The format requested is dd.MM.yyyy, year comes last which is different from the format you added. I think just looking into the date separator is not enough anymore. We gotta look for the whole format and use that instead.

On Sat, Feb 22, 2020, 9:37 PM Brady Trainor notifications@github.com wrote:

Closed #48 https://github.com/bradyt/cone/issues/48.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bradyt/cone/issues/48?email_source=notifications&email_token=ADCRQOZCZZ4O7VMBKMS7EI3REHOOFA5CNFSM4JBZGEZ2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOW2JWZYI#event-3063114977, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCRQO352W63VA4V6EUA4ZTREHOOFANCNFSM4JBZGEZQ .

bradyt commented 4 years ago

The format requested is dd.MM.yyyy, year comes last which is different from the format you added. I think just looking into the date separator is not enough anymore. We gotta look for the whole format and use that instead. […]

@justinjk007 I feel I replied to this aspect with the following:

If you would like a feature added, can you file a new issue, with a title like, "Write dates as day month year", little-endian dates or whatever?

In other words, I feel this issue deserves its own thread. Anyone is welcome to start that thread.