search

mikehardy / thunderlink

Link to your Thunderbird emails!
Mozilla Public License 2.0
42 stars 14 forks source link

Format options/alternatives for <time> and <sender> #49

Closed novoid closed 3 years ago

novoid commented 4 years ago

I'd like to ask for format options.

1/10/2020 - 10:46:18 AM Karl Voit <Karl.Voit@example.com>: This is a test email is my current result when I'm using <time> <sender>: <subject>.

I'd love to have this instead: 2020-01-09 Karl Voit: This is a test email

Therefore:

  1. an ISO date (instead of locale-formatted long time-stamp) and
  2. the sender name (without email address).

Thanks a lot.

MarioKusek commented 4 years ago

If we have just one tag for a time then I think that it should be either local one as it is now or ISO8601. If there are more tags for a time then this is a possible proposal:

MarioKusek commented 4 years ago

Regarding sender name. Here is my opinion similar to the time. If we have one tag then it should be as it is now with name and email. If we have more tags then a possible proposal is:

mikehardy commented 4 years ago

I think the proposals are good in either direction, and I would defer to you @MarioKusek or anyone that wanted to do the work (I call this rule "Implementor's choice" -> if you are going to implement it I will get out of the way :-) ), and I would happily merge it

novoid commented 4 years ago

The proposal seems concise and consistent. That would give the user the best options to choose from.

dnicolodi commented 4 years ago

My personal take on the issue was to use a place holder in the formal <time:format-string> where format-string is a strftime() like format string. I thought I would have been easy to implement... however I quickly learnt that JS does not have a sane datetime formatting library, and I gave up.

MarioKusek commented 4 years ago

@dnicolodi I agree with you that flexible format would be the best solution but as you said JS has problems with date formating. Parsing format string would also be more complicated but doable.

jamesquilty commented 4 years ago

@MarioKusek I support the addition of an ISO date and time format option to thunderlink, I can see myself using it in some contexts. That said, I filed #63 because - from my point of view - all of a sudden the <time> format behaviour changed. Local time formatting was, and is, context-appropriate for most of my uses. So your decision in commit 8ce1350d552551d6c8d1a36fef38253029332f3b to replace the existing format obtained with <time> that everybody uses (because there was no other option) with another format came as an unpleasant surprise.

@mikehardy I suggest that the old behaviour of <time> giving localeTime format, established in previous versions, be restored as soon as possible and, instead of a new <localeTime> tag to restore the previous behaviour, a new <ISOtime> tag for ISO formatting be created instead. It's better to only temporarily change an end-users' configuration than to arbitrarily make a permanent change. Would you be willing to make the changes?

jamesquilty commented 4 years ago

In https://github.com/mikehardy/thunderlink/issues/63#issuecomment-636278755 @MarioKusek wrote:

If that is your proposal, we will use <time> for local time, and <ISOtime> for just hours, minutes ... This is strange to me. It has a semantic mismatch in tags and I don't think this is the correct way to go.

The existing semantic mismatch is that the tag <time> resolves to the full date and time string (year, month, day, hours, minutes, seconds) and <dateTime> resolves to just the time string (hours, minutes, seconds).

This mismatch was created by you in commit https://github.com/mikehardy/thunderlink/commit/8ce1350d552551d6c8d1a36fef38253029332f3b lines 160 and 162 and subsequently merged. No, it wasn't the correct way to go, and it is in conflict with what you said you'd implement in https://github.com/mikehardy/thunderlink/issues/49#issuecomment-573385712 above. Perhaps this semantic mismatch is a bug to be fixed?

dnicolodi commented 4 years ago

Is there an half good JS package that implements something compatible with strftime()? If so I think it would be best to add it as a dependency and have a syntax like the one I proposed a while ago. For not breaking backward compatibility it could be <datetime:format> where the format part is passed verbatim to the strftime() equivalent.

dnicolodi commented 4 years ago

I found two implementations that do not seem too bad at a quick look: https://github.com/thdoan/strftime and https://github.com/samsonjs/strftime

mikehardy commented 4 years ago

PRs happily accepted but please backwards compatible :sweat_smile:

MarioKusek commented 4 years ago

@mikehardy I have compared what I proposed and what is implemented. And there are differences. I proposed:

  • <time> should write ISO8601 time of day (format hh:mm:ss.sss). I don't know if we need timezone here. If we want to have a complete solution the timezone should be present.
  • <date> should write ISO8601 date (format YYYY-MM-DD)
  • <dateTime> should write full ISO8601

And implementation is the following:

 result = result.replace(/<time>/ig, isoDate);
 result = result.replace(/<date>/ig, dateString);
 result = result.replace(/<dateTime>/ig, timeString);

This was my mistake in implementation ☹️ Thanks to @jamesquilty for pointing it out here.

In order to fix this we have two options:

@mikehardy What do you think we should do?

novoid commented 4 years ago

Maybe I overlook something here. IMHO it'd be a non-breaking change, yes, but it is easily spotted and can easily fixed by the people. I favor fixing the semantics as soon as possible to avoid even more "pain".

Is there a version 1.3 or 2.0 in the pipeline? Maybe combining this fix with a major release change is a viable approach?

mikehardy commented 4 years ago

I don't think the add-ons have any kind of semantic versioning gate on updates so really there's no way for the user to know that the implementation is going to start returning different values. If different input returns different value, it's breaking, to me.

As people trying to maintain this thing we have to be okay with breaking change though, so if we did a 2.0 and it was documented very clearly (a breaking changes section very near the top of the doc with a very short, very clear list of exactly what changed and what action to take, possibly linked to a larger separate page somewhere with full details if people wanted), then let's do it.

Honestly I love this module and I want to shepherd it (in concert with all interested people here) well, but I have no real plans for it other than to keep it working. It has just over 1000 users based on the add-ons stats server so we shouldn't break it but this isn't really high-impact software compared with the other projects I work on with 1million+ installs :-). So let's maintain perspective and just not break people's daily workflow without good reason is my plan

novoid commented 3 years ago

So there won't be further format options without a v2.0? :cry:

mikehardy commented 3 years ago

Oh - I was just cleaning up stale issues in general. Right now thunderlink appears to be in a middle place where it doesn't work (yet) on modern thunderbird but there is potential for future support. I don't see the point in spending time on something that is not possible to support in current versions though, sorry.