Frederick888 / external-editor-revived

External Editor Revived is a Thunderbird MailExtension which allows editing emails in programs such as Vim, Neovim, Emacs, etc.
GNU General Public License v3.0
113 stars 6 forks source link

feat(ext): Warning when "/path/to/temp.eml" is missing #118

Closed lorenzleutgeb closed 1 year ago

Frederick888 commented 1 year ago

Thanks for the idea!

I updated it a little to show the message only when "/path/to/temp.eml" is missing from the template. I think this is of a bit less noise.

lorenzleutgeb commented 1 year ago

I do understand your reason for showing the message only when it is missing. However, it somehow misses my motivation for the original PR: When I read something like /path/to/temp.eml, it is not obvious to me that this string will be replaced. I read things like /path/to/... in tutorials/documentation, when from the context it is clear that I should manually replace the path. In the case of external-editor-revived, I would expect something like $EMAIL_FILE to be used as a placeholder. A variable name, that looks a little more formal than /path/to/temp.eml. My intention was to inform the user right then and there, when the string /path/to/temp.eml actually is shown. When I first opened the settings and saw this string, I immediately was confused, because I thought I should manually replace it. Now, if you only show the message when the string is absent, then it's already too late. People that remove the string already were confused, and now they have to revert their change.

I think that the information that /path/to/temp.eml will be replaced should always be visible. The information that /path/to/temp.eml must be present could possibly only be shown when it is indeed missing.

Frederick888 commented 11 months ago
  1. Still there is no need to show any of these if the user is not using Custom
  2. I agree there are probably better choices than /path/to/temp.eml. But this is kinda subjective and I didn't have a luxury to run a survey... Btw $EMAIL_FILE (or dollar-anything) would not be a wise choice either, as it will stop users from actually using this environment variable.
  3. We can support a new default placeholder along with /path/to/temp.eml together. But again since it's subjective and I don't think it's worth the effort -- implementation-wise or maintenance-wise. Not to mention it'll trigger a very confusing default template migration prompt for non-Custom users.
  4. I beg, especially Custom users, to just read the Wiki lol... If you find the wording in Wiki confusing, I'm open to suggestions. People who don't read / get confused will (frustratingly) skip reading things / get confused no matter where you put them.