-
Notifications
You must be signed in to change notification settings - Fork 320
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
Added new options to use arbitrary time format. #124
Added new options to use arbitrary time format. #124
Conversation
Hmm, why are you using an if inside a case? Seems odd. |
What do you mean? |
So this is a good idea, but instead of creating two options here, could we just check if |
I think I need to add some description about this new option, but couldn't find out where to write.
You are right. So, how about this one ? (deleted the
|
I confirmed that the code above works. Btw, the time related options are hard to configure and adding this new option would make them unnecessary. (but deleting them would break backward compatibility...) What do you think about this? |
Does the code work with time zones? Not sure if you can add it with the % syntax, or if you need to add the ${timezone} variable. |
Also, for backwards compatibility, let's keep the existing options at least for the time being. Maybe in the future they can be removed. If you could squash all your commits into one that would be useful. Also updating your PR title. |
Yes, you need to document the options in INSTALL.md. A baby should be able to follow them ;) |
Yes, you can add timezones like this:
Should I change to respect the
Understood. |
IMO, yes. |
Hi, will this PR be merged in the future? |
307ecc4
to
597f7be
Compare
Added new options because I wanted to use my original time format.
dracula-arbitrary-time-format
: true/false, default to falsedracula-time-format
: time format, default to "%Y-%m-%d(%a) %H:%M"Where should I write readme??