Skip to content
This repository has been archived by the owner on May 3, 2023. It is now read-only.

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

Closed
novoid opened this issue Jan 10, 2020 · 16 comments
Closed

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

novoid opened this issue Jan 10, 2020 · 16 comments

Comments

@novoid
Copy link

novoid commented Jan 10, 2020

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
Copy link
Collaborator

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:

  • <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

@MarioKusek
Copy link
Collaborator

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:

  • <sender> should be as it is (name and email) - Karl Voit <Karl.Voit@example.com>
  • <senderName> should be just name - Karl Voit
  • <senderEmail> should be just an email - Karl.Voit@example.com

@mikehardy
Copy link
Owner

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
Copy link
Author

novoid commented Jan 12, 2020

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

@dnicolodi
Copy link

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
Copy link
Collaborator

@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
Copy link

@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 8ce1350 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
Copy link

In #63 (comment) @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 8ce1350 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 #49 (comment) above. Perhaps this semantic mismatch is a bug to be fixed?

@dnicolodi
Copy link

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
Copy link

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
Copy link
Owner

PRs happily accepted but please backwards compatible 😅

@MarioKusek
Copy link
Collaborator

@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:

  • leave illogical tags and have backward compatibility (and of course fix documentation)
  • fix code according to the original proposal

@mikehardy What do you think we should do?

@novoid
Copy link
Author

novoid commented May 31, 2020

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
Copy link
Owner

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
Copy link
Author

novoid commented Dec 4, 2020

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

@mikehardy
Copy link
Owner

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants