wangp / bower

A curses terminal client for the Notmuch email system
Other
119 stars 11 forks source link

Feature: Composing multipart/alternative #58

Closed xunam closed 3 years ago

xunam commented 4 years ago

Regarding the possibility of composing HTML messages (alluded to in another issue: https://github.com/wangp/bower/issues/28#issuecomment-619321586), a recent release of Mutt introduced this ability. The approach looks reasonably simple and consistent, it basically consists in piping the plain text content through a configurable external tool that produces the alternative content. That could be a source of inspiration…

seifferth commented 3 years ago

+1 for this. I actually already played around with this idea a while ago. As the text editor is configurable, I simply wrapped the call to vi so that it would strip any multipart html on load and add it on save. It didn't work well with bower, though, message preview broke my multipart message. Maybe one could reuse the message view for usual messages for previewing messages to be sent? This would even enable some sort of html preview.

seifferth commented 3 years ago

So I started adding this feature. Here's some further notes on it. Suggestions and feedback are welcome.

Edit: Tick some checkboxes in the list below.

Notes on alt_html_filter

Things that aren't really solved yet

Open wishes

Crazy thoughts

wangp commented 3 years ago

That's looking pretty good. Perhaps the HTML alternative should be only be generated on demand, e.g. on the compose 'H' could toggle whether to run the HTML filter. (This feature can be added later.)

seifferth commented 3 years ago

Thanks.

I also gave toggling the filter some thought. I don't really see the use case for toggling it on a per message basis. I. e., at least in my case, there is some sort of structure behind when I want to use the filter. E. g. I have this one sender address I want to send HTML messages from. Or I have that one friend who uses some horribly outdated Smartphone to read my emails, so I want to adjust the style of the alternative so that my emails look nice on that device. In general, I want to be consistent about the application of those rules and not forget to toggle multipart generation on every second email I send.

I guess there would be two ways to get such rules working.

  1. We could do something similar to the per-account sendmail command. I. e. a per-account filter command. But what if I want to run the filter for certain recipients, no matter which sender address I use.
  2. We could expose the headers to the filter so that the filter can decide what to do. This keeps a lot of complexity out of bower itself, which I find desirable.

The second option would also allow users to make the filter as simple or as complex as they want (or need) it to be. Anything is possible, from just always run it or if test "$from" = "My Multipart Address <myself@multipart.email>"; then pandoc -f markdown -t html; fi up to inventing a custom sendmail-style turing-complete programming language for writing the filter configuration.

Additional thoughts

One additional thought would be that we might want to add a custom use filter/don't use filter header. While I will mostly be matching from and to addresses against regular expressions to determine if I want to use a filter or not, others may find it more convenient to include the following in the header:

Bower-Filter: true

Or don't use the filter:

Bower-Filter:

And then simply wrap pandoc in a shell script:

#!/bin/sh

if test "$bower_filter"; then
    pandoc -f markdown -t html
fi

Someone else might actually want to use that same header as such:

Bower-Filter: pandoc -f markdown -t html
#!/bin/sh

$bower_filter

(I didn't really check how shell escapes and stuff work for the alt_html_filter, but it might even be possible to rewrite the above example entirely inside bower.conf as alt_html_filter = $bower_filter.)

We could strip the Bower-Filter header before actually sending the email, just like we strip the Bcc header. (We do strip the Bcc header, right?) Or we could strip any header that starts with Bower- (or some other prefix) and update the environment generation to include Headers ^ h_rest (there is already a TODO comment for that one, although I don't really consider it high priority as I am currently happy enough with accessing $from and $to).

seifferth commented 3 years ago

Just a quick update on the additional thoughts: I exposed Headers ^ h_rest as well and both the examples suggested above do work. Even specifying text_alt_filter = $bower_filter directly in bower.conf sort of works (it does fail if the command contains quotes).

I guess the easiest way to make the filter configurable on a per-message basis is to specify it as:

text_alt_filter = if test "$bower_filter"; then <whatever the filter command is>; fi

I didn't strip any headers yet, and I guess that would also be an issue that deserves more thought and broader discussion.

wangp commented 3 years ago

I was hoping to keep it simple :) If we have two config keys, alt_html_filter and alt_html_by_default, users who want to send HTML alternatives most of the time can set alt_html_by_default = true (and remember to write proper markdown). Users who only want to send HTML alternatives occasionally can set alt_html_by_default = false. And the setting could be changed for a particular message by pressing 'H'.

That might not be convenient enough for your examples. I suggest separating the decision to add a HTML alternative from the filter itself. alt_html_by_default or another key could specify a command that takes the recipient addresses as arguments, and return exit status 0 for true. Something like that.

I don't think the alt_html_filter should receive any headers as environment variables.

seifferth commented 3 years ago

I totally agree to keeping it simple ;) And I am well aware that the filter I am using is way over the top. So that is basically why I tried to keep as much of it out of bower itself as I could.

About environment variables: I would still like the filter to be (at least potentially) aware of the email it is supposed to convert, without forcing that awareness on it either. That is basically why I used environment variables. It just seemed to be the easiest way of exposing information without changing the call to the filter itself. Note that the proposal works just as well with my over the top filter as it does with a simple invocation of pandoc. Pandoc doesn't know anything about bower, email headers and the like. And why should it. (Also note that plain pandoc doesn't work with the way they implemented the filter in mutt, since they require a somewhat customised output format.) In summary, I would still like to see the environment variable solution in bower. We might talk about which headers to expose, but in a way, simply exposing all of them is the simplest possible solution. I did feel a need to normalise the header names, since Reply-To would work in most programming languages, but isn't valid bash syntax for a variable. And bash is still sort of a reference language for scripts, as I see it. Am I missing something about the environment variable issue here?

Introducing a second command that decides whether or not to use the filter is an option. But the way I see it, that would increase complexity, rather then prevent it. It is one more callout, for instance, and in case the command is meant to reach a decision based on header values, we would still need to find a way to make it aware of those. We could pass them as arguments, sure, but that would mean that users need a customised command that understands those arguments. And then there is still the issue of how to expose them. Email headers are pretty much an arbitrary map from string to string. Do we pass some of them as --from "<Some address>" --to"<Some address>" etc. Or do we pass them as <filter_decision_command> "From: Some address" "To: Some address". In the last case, how do we handle the fact that headers are, in theory, case-insensitive? Also note that the second option is pretty close to from="Some address" to="Some address" <filter_decision_command>, which is what I decided to go for ;)

If you care about it, I could add an option to toggle the filter via H and make that option configurable. Maybe something like use_alt_html_filter = {never,always,manual,auto}. never and always would be pretty straightforward, I guess. manual could enable that H key you brought up, and auto might be set to some sane default that works well for most people, such as use the filter when replying to HTML email, don't use it when replying to plain text email. The way I see it, this would, in a way, still make the filter less simple than it is at the moment. But it might be a solution where both of us get what we want.

Edit: If there was an H keybinding, I would actually like to be able to disable it for myself. I find it hard enough to attach all the attachments I mention in the text already, and I really don't want to have to think about whether the filter is run or if I might have accidentally pressed a key I didn't mean to press. If use_alt_html_filter is set to always, H could just print a warning informing the user that nothing will happen and that they should check their configuration if this isn't what they expected. The same might be a good idea for never. manual might be off by default and maybe we could add an option default, which would mean on by default. (Anyway, the one I actually want to have is always, the rest is pretty much up to you.)

wangp commented 3 years ago

About environment variables: I would still like the filter to be (at least potentially) aware of the email it is supposed to convert, without forcing that awareness on it either. That is basically why I used environment variables. It just seemed to be the easiest way of exposing information without changing the call to the filter itself. Note that the proposal works just as well with my over the top filter as it does with a simple invocation of pandoc. Pandoc doesn't know anything about bower, email headers and the like. And why should it. (Also note that plain pandoc doesn't work with the way they implemented the filter in mutt, since they require a somewhat customised output format.) In summary, I would still like to see the environment variable solution in bower. We might talk about which headers to expose, but in a way, simply exposing all of them is the simplest possible solution. I did feel a need to normalise the header names, since Reply-To would work in most programming languages, but isn't valid bash syntax for a variable. And bash is still sort of a reference language for scripts, as I see it. Am I missing something about the environment variable issue here?

The main issue is with exposing all the headers.

There's not really an issue with exposing a small, fixed set of headers.

Introducing a second command that decides whether or not to use the filter is an option. But the way I see it, that would increase complexity, rather then prevent it.

I think it's pretty much necessary for manual toggling, see below.

If you care about it, I could add an option to toggle the filter via H and make that option configurable.

Yes, there must be a way to toggle the option at runtime, otherwise it would be necessary to modify either bower.conf or the filter script to turn on HTML alternatives for a single message. Let me run through my design idea with use cases.

User who only wants HTML alternative occasionally

  1. set alt_html_filter to pandoc or something; note this is a generic command that is not specific to any user
  2. set alt_html_by_default = false
  3. compose a message (remember to write in markdown), then leave the text editor
  4. on entering the compose screen for the first time, bower sees that alt_html_by_default is false so does nothing
  5. user presses H
  6. bower runs the alt_html_filter and shows the HTML alternative in the pager

At step 6, the alt_html_filter command must not make the decision whether to generate HTML or not; the user already decided that in step 5. Separating the two commands is necessary, otherwise we need set an environment variable or something to tell the script to ignore its normal decision making. And every user's script would need to take that environment variable into account.

User who wants HTML alternative always

  1. set alt_html_filter to the same command as before
  2. set alt_html_by_default = true (it can be the default value when alt_html_filter is set)
  3. compose a message, then leave the text editor
  4. on entering the compose screen for the first time, bower sees that alt_html_by_default is true
  5. bower runs the alt_html_filter and shows the HTML alternative in the pager

User who wants HTML alternatives for specific recipients

  1. set alt_html_filter to the same command as before
  2. set alt_html_by_default to a script that implements their complex rules
  3. compose a message, then leave the text editor
  4. on entering the compose screen for the first time, bower calls the alt_html_by_default script
  5. if the script returns true, bower runs the alt_html_filter and shows the HTML alternative in the pager

Edit: If there was an H keybinding, I would actually like to be able to disable it for myself.

We would write an info message "Generated HTML alternative" or "Removed HTML alternative", and combined with the HTML preview, I think it would be hard to miss. What do you think?

seifferth commented 3 years ago

Thanks a lot for the elaborate clarification.

Toggling the filter

I see we came up with different solutions for how to turn HTML generation on and off. I proposed to delegate that decision to an external program while you want to use a manual toggle (with a customisable default value). I guess both cases have their pros and cons, so I suppose it would be a good idea to offer users a choice between them.

As far as the manual toggle is concerned, I do like your proposal. I don't quite like the idea of setting alt_html_by_default to a customised command, though. In my opinion, this would still be too tightly integrated with bower. What I would like to see is a choice between make the decision in bower and completely delegate the decision to an external program. Setting a default value is not delegating it decidedly enough for my taste.

Of course, if I completely delegate the decision to an external command, that command needs to be able to reply to its call in three ways, rather than just with success or failure. The way I see it, there are two ways of achieving this:

  1. (Your proposal) Call two commands, c1 and c2. Combining these calls gives us three options (excuse my pseudo-code):
    1. c1 && c2 exits zero
    2. c1 && c2 exits non-zero (with c2 failing)
    3. c1 exits non-zero
  2. (My proposal) Call one command, but take both the exit code and stdout into account:
    1. Exit code zero, data on stdout
    2. Exit code non-zero
    3. Exit code zero, nothing on stdout

Note that I aligned the combinations so that equivalent combinations have corresponding numbers. I guess one could argue that my solution is kind of hackish, since it interprets the data on stdout in two ways:

  1. As the HTML equivalent of what was passed in on stdin.
  2. As the filter's reply to the question: "Am I supposed to add an HTML alternative to this email?"

Of course this overloading means that there is one possible response that the filter simply cannot express: "Please add the empty string as an HTML alternative." I still went ahead with overloading stdout because I didn't consider this to be a response we would want any filter to give. The only case in which this might come up is if the message itself is empty. And I would argue that empty messages simply shouldn't be multipart. There is no point in expressing nothing in two different markups, is there? (We could, of course, still add a warning that the filter returned nothing on stdout, so that users know why they don't see any HTML preview.)

Note: Even if the message is empty, the filter could still produce a html representation of nothing that bower would add. As it happens, this is what pandoc does:

$ printf '' | pandoc -f markdown -t html

$

For empty input, pandoc produces a single newline. As far as the current implementation of alt_html_filter is concerned, a single newline is still data rather than nothing, so it would add it as an HTML alternative. I guess we could argue about whether we actually want bower to do this. I myself am actually leaning towards a solution where bower would ignore any whitespace-only output the filter produces. But I guess that would be easy to add if you agree with me.

As far as the use cases are concerned, here is how it would look like in my proposal:

User only wants HTML alternative occasionally

  1. Set alt_html_filter to pandoc or something.
  2. Set use_alt_html_filter = manual.
  3. Compose a message, maybe write markdown, then leave the text editor.
  4. On entering the compose screen for the first time (see below), bower sees use_alt_html_filter is manual, so does nothing.
  5. User presses H.
  6. Bower runs the alt_html_filter and shows the HTML alternative in the pager (if there is one). Ideally, bower would show the preview in the same way it shows alternatives in received messages. I. e. the user can select [-- text/plain --] by pressing v and then toggle preview with z.

At step 6, the alt_html_filter command should not make the decision whether to generate HTML or not. It is the user's responsibility to configure alt_html_filter appropriately. In the classic Unix fashion, bower gives users enough rope to hang themselves if they so desire. The filter is an external command. For all we know, it could be cat ~/.ssh/*. And if that is what the user tells bower to do, bower should just do it. There is no technical reason why it couldn't, so why shouldn't it.

User wants HTML alternative most of the time

  1. Set alt_html_filter to pandoc or something.
  2. Set use_alt_html_filter = default.
  3. Compose a message.
  4. On entering the compose screen, bower sees that use_alt_html_filter is default.
  5. Bower runs the alt_html_filter and shows the preview (if there is one).
  6. The user decides against using HTML this time and presses H.
  7. Bower strips the HTML alternative and doesn't add it again until it sees another H. If the message is sent out without toggling the filter on again, it is sent out as text/plain.

User wants HTML alternative always

  1. Set alt_html_filter to pandoc or something.
  2. Set use_alt_html_filter = always.
  3. Compose a message.
  4. On entering the compose screen, bower sees that use_alt_html_filter is always.
  5. Bower runs the alt_html_filter and shows the preview (if there is one).
  6. The user presses H for whatever reason.
  7. Bower tells the user that this won't work. The user just told it to always produce multipart messages. Always means always, also this time. If this isn't what the user wants, they should change their configuration to reflect what they actually want. (Probably use_alt_html_filter = default or something. Again, it is the user's responsibility to choose the configuration option that actually reflects their preferences.)

User wants something more complicated

OK, if you want something complicated and customised, bower won't do it for you. Bower is meant to be kept simple. But you are free to implement your own solution and tell bower to use that as an external command.

  1. Set alt_html_filter to some customised command.
  2. Set use_alt_html_filter = always.
  3. Compose a message.
  4. On entering the compose screen, bower sees that use_alt_html_filter is always, so it knows that the decision about whether or not to include the multipart message belongs to the filter.
  5. Bower runs the alt_html_filter and shows the preview (if there is one). It interprets the filter response in the three-fold way outlined above:
    1. Here's the HTML part: Exit code zero, HTML on stdout. Bower just adds this as usual.
    2. Houston, we have a problem: Exit code non-zero, stdout doesn't matter. Bower informs the user that there was a problem so they can have a word with their filter.
    3. Look, bower, I did what I was supposed to do, and, actually, there is no HTML this time: Exit code zero, nothing on stdout. Bower just accepts this response and doesn't add anything. This is clearly visible in the preview.

User wants something complicated that still respects the H key

Now things really get a little messy. The user apparently wants to use a complicated solution, but they also want it to be tightly integrated with bower. I am not sure to what extend bower should handle this case. With the somewhat simple solution outlined above, the following is still possible:

  1. Toggle the filter off in some cases:
    1. Set alt_html_filter to some customised command.
    2. Set use_alt_html_filter = default.
    3. Compose a message.
    4. On entering the compose screen, bower runs the alt_html_filter and shows the preview (if there is one).
    5. The user can now use H to turn the custom filter off for this one message.
  2. Toggle the filter on in some cases:
    1. Set alt_html_filter to some customised command.
    2. Set use_alt_html_filter = manual.
    3. Compose a message.
    4. On entering the compose screen, bower doesn't run the alt_html_filter.
    5. The user can now use H to turn the custom filter on for this one message.

Of course there is no one filter that works well for both cases. If the filter is meant to be on by default, it is probably supposed to use a rather different strategy for bailing out of HTML generation than if it is meant to be off by default. But again, it is the user's responsibility to get this right. If they want to use a highly customised filter, they need to make sure it works well for them.

The one thing that is hard to solve in this case is a filter that is on by default, bails out of HTML generation for most messages, but still accepts H as a toggle. This time not as a toggle of the filter itself, but rather of its default strategy. I guess I could go on to propose a solution that would provide for this special case, but actually, I don't think bower should handle this. At least not until there is someone who actually wants to use this feature. And even if there were, I am not sure if it would be worth it.

Entering the compose screen for the first time

I don't think the compose screen should behave differently the first and second time around. What if I forget to fill in the To: field the first time? Then I enter the compose screen, I notice, go back to the editor, fix it, and reenter. The result should be the same as if I had gotten it right the first time.

Environment variables

The main goal I wanted to achieve is to have all the information a user enters into their text editor when composing a message exposed to the filter. I see the filter as a post-processing step of what the user does in the text editor, so I felt that it should also receive the full input. I also played around with a different solution before: Piping the whole file (including headers) to the filter. The problem with this solution is that the filter needs to accept an email message as input, rather than just some text formatted in markdown. This means that standard utilities like markdown or pandoc don't work out of the box. And, in an effort to keep it simple, I felt that they should.

You have to explain the mapping from header names (case insensitive, contains '-') to environment variables.

Granted, but I guess that explanation could be as simple as In-Reply-To becomes in_reply_to. Other headers work accordingly. I suppose most people should find this mapping rather intuitive. If you feel the need, we could add a cautionary note that users should use only one of In-Reply-To, in-reply-to, IN-REPLY-TO, IN_REPLY_TO etc. in their headers or bower may behave in an undefined way. But then, apart from IN_REPLY_TO, this is already the case, right?)

Introduces potential name conflicts. Sure it's unlikely, but if we can avoid the problem altogether.

I see what you mean. Unfortunately, this problem doesn't depend on bower alone but also on the filter, which could be any command. It also exists if we expose just one environment variable. Say we just expose to='"Someone" <someone@provider.mail>' and for some reason the filter decides to take this as the desired output format and fails. Say we prefix everything with bower_, but the filter takes bower_to to be an upload directory for some package manager. There is simply no way to use environment variables without introducing potential name conflicts. Ever. That being said, as far as I know, widely-scoped environment variables are usually uppercase, which is why I went for lowercasing the header values. I guess the best way to handle name conflicts is to handle them on a per-filter basis as they come up:

$ from="example" bash -c \
>   'alt_from="$from" from="" bash -c env' \
>   | grep -P '^(alt_from|from)'
from=
alt_from=example

There's not really an issue with exposing a small, fixed set of headers.

Would still be fine with me. I could revert the Headers ^ h_rest commit if that makes you feel better ;) As mentioned earlier, if I get from and to (and maybe cc and bcc at some point) I am happy enough.

wangp commented 3 years ago

For empty input, pandoc produces a single newline. As far as the current implementation of alt_html_filter is concerned, a single newline is still data rather than nothing, so it would add it as an HTML alternative. I guess we could argue about whether we actually want bower to do this. I myself am actually leaning towards a solution where bower would ignore any whitespace-only output the filter produces. But I guess that would be easy to add if you agree with me.

Yes, I think we should treat whitespace-only output as a signal not to generate an HTML part. (The pandoc command should probably pass --standalone anyway.)

As far as the use cases are concerned, here is how it would look like in my proposal:

User only wants HTML alternative occasionally

  1. Set alt_html_filter to pandoc or something.
  2. Set use_alt_html_filter = manual.

That's fine.

User wants HTML alternative most of the time

  1. Set alt_html_filter to pandoc or something.
  2. Set use_alt_html_filter = default.

That's fine.

User wants HTML alternative always

  1. Set alt_html_filter to pandoc or something.
  2. Set use_alt_html_filter = always.

I still don't see the need for always, but it's fine.

User wants something more complicated

OK, if you want something complicated and customised, bower won't do it for you.

That's fine for me. I didn't want anything except the manual option anyway :)

User wants something complicated that still respects the H key

The one thing that is hard to solve in this case is a filter that is on by default, bails out of HTML generation for most messages, but still accepts H as a toggle. This time not as a toggle of the filter itself, but rather of its default strategy. I guess I could go on to propose a solution that would provide for this special case, but actually, I don't think bower should handle this. At least not until there is someone who actually wants to use this feature. And even if there were, I am not sure if it would be worth it.

Great, let's forget it then.

Entering the compose screen ~for the first time~

I don't think the compose screen should behave differently the first and second time around. What if I forget to fill in the To: field the first time? Then I enter the compose screen, I notice, go back to the editor, fix it, and reenter. The result should be the same as if I had gotten it right the first time.

I was a bit wary about that part of the design but with the manual override it seemed acceptable.

Environment variables

The main goal I wanted to achieve is to have all the information a user enters into their text editor when composing a message exposed to the filter. I see the filter as a post-processing step of what the user does in the text editor, so I felt that it should also receive the full input. I also played around with a different solution before: Piping the whole file (including headers) to the filter. The problem with this solution is that the filter needs to accept an email message as input, rather than just some text formatted in markdown. This means that standard utilities like markdown or pandoc don't work out of the box. And, in an effort to keep it simple, I felt that they should.

Agreed, we don't want the tools to have to parse RFC822-style messages (or more correctly, a custom format that superficially looks like RFC822).

Introduces potential name conflicts. Sure it's unlikely, but if we can avoid the problem altogether.

I see what you mean. Unfortunately, this problem doesn't depend on bower alone but also on the filter, which could be any command. It also exists if we expose just one environment variable. Say we just expose to='"Someone" <someone@provider.mail>' and for some reason the filter decides to take this as the desired output format and fails. Say we prefix everything with bower_, but the filter takes bower_to to be an upload directory for some package manager. There is simply no way to use environment variables without introducing potential name conflicts. Ever.

Exactly.

That being said, as far as I know, widely-scoped environment variables are usually uppercase, which is why I went for lowercasing the header values.

I'm undecided how to name the environment variables. Some ideas:

  1. To, From -- title case is how the header names are usually presented
  2. TO, FROM -- upper case is the normal convention for environment variables
  3. BOWER_To, BOWER_From -- add some prefix to make it clear where the variables come from and avoid conflicts
  4. BOWER_TO, BOWER_FROM -- same but less ugly

We already have PART_CONTENT_TYPE and PART_CHARSET for other filter commands so some consistency would be nice. Therefore I'm leaning towards either (2) or (4).

edit: obviously this is just bikeshedding and I don't wish it to distract from the rest of the work. We can always change things later.

There's not really an issue with exposing a small, fixed set of headers.

Would still be fine with me. I could revert the Headers ^ h_rest commit if that makes you feel better ;) As mentioned earlier, if I get from and to (and maybe cc and bcc at some point) I am happy enough.

Please do.

seifferth commented 3 years ago

Great. I guess the main points are clear then.

The pandoc command should probably pass --standalone anyway.

I am not quite sure about this one. Running pandoc with --standalone produces 840 characters of boilerplate and a warning that: "This document format requires a nonempty element. Defaulting to '-' as the title." Looks like a bit of an overkill to me. At the very least, pandoc would need to use a custom HTML template for emails. But then, I do believe that HTML snippets should usually work. I have sent one out already (including a <code><style>[...]</style></code> section that would belong in <code>head</code>, but there was neither <code>head</code>, nor <code>body</code>, nor <code>html</code> for that matter) and it was rendered nicely by my friend's device.</p> <blockquote> <p>I'm undecided how to name the environment variables. [...] We already have <code>PART_CONTENT_TYPE</code> and <code>PART_CHARSET</code> for other filter commands so some consistency would be nice.</p> </blockquote> <p>Agreed. I didn't think about consistency with those yet, I have to admit. <code>FROM</code> and <code>TO</code> would be OK as well, I guess. Maybe something like <code>HEADER_FROM</code> or <code>HEAD_FROM</code> would be more in line with the <code>PART_</code> naming scheme? But as you mentioned, this is really just a minor detail.</p> <h1>One more thought on defaults</h1> <p>Maybe we should use the following configuration as our default values:</p> <pre><code>alt_html_filter = pandoc -f markdown -t html use_alt_html_filter = manual</code></pre> <p>That way, multipart generation would just work out of the box if pandoc is installed on the system. No configuration needed, everything is just one <code>H</code> away. (This is also just a minor detail, of course.)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/seifferth"><img src="https://avatars.githubusercontent.com/u/43514227?v=4" />seifferth</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>So I started moving parts from <code>src/thread_pager.m</code> to <code>src/pager.m</code> in order to make viewing composed multipart messages easier. Here's some points where I could use some feedback:</p> <ul> <li>I disabled the toggle-expanded connotation of <code>o</code> for now, as this decoupling made it easier to move parts around. <code>z</code> and <code>Z</code> still work, so maybe we don't need <code>o</code> for toggle-expanded? (If someone actually cares about it, I could also add this again later.)</li> <li>I might have broken encrypted email display. I don't really use email encryption, so I didn't have any test data at hand. Maybe someone could check if displaying encrypted parts in the thread pager still works on the feature branch. Especially decrypting something by pressing <code>z</code>or <code>Z</code>. If that is what the code does ;) I have never used this feature, just seen the code.</li> <li>The keybindings for <em>subject</em> and <em>save part</em> conflict now that saving parts of composed messages is possible. I changed the keybinding for <em>subject</em> to <code>S</code> on the feature branch, and I also changed the keybinding for <em>sign</em> to <code>I</code> (as there can only be one <code>S</code>). I am open to suggestions as to which keybindings to use, but I feel that <em>save part</em> should use the same key in both thread pager and compose and <code>s</code> seems to be a good fit.</li> </ul> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/wangp"><img src="https://avatars.githubusercontent.com/u/936637?v=4" />wangp</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Thanks for your work so far!</p> <blockquote> <ul> <li>I disabled the toggle-expanded connotation of <code>o</code> for now, as this decoupling made it easier to move parts around. <code>z</code> and <code>Z</code> still work, so maybe we don't need <code>o</code> for toggle-expanded? (If someone actually cares about it, I could also add this again later.)</li> </ul> </blockquote> <p>That's fine. I forgot about that function of <code>o</code> myself.</p> <blockquote> <ul> <li>The keybindings for <em>subject</em> and <em>save part</em> conflict now that saving parts of composed messages is possible. I changed the keybinding for <em>subject</em> to <code>S</code> on the feature branch, and I also changed the keybinding for <em>sign</em> to <code>I</code> (as there can only be one <code>S</code>). I am open to suggestions as to which keybindings to use, but I feel that <em>save part</em> should use the same key in both thread pager and compose and <code>s</code> seems to be a good fit.</li> </ul> </blockquote> <p>Hmm, I quite like <code>s</code> for subject and <code>S</code> for sign. Perhaps I'm just used to it. Is save part necessary on the compose screen?</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/wangp"><img src="https://avatars.githubusercontent.com/u/936637?v=4" />wangp</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <blockquote> <ul> <li>I might have broken encrypted email display. I don't really use email encryption, so I didn't have any test data at hand. Maybe someone could check if displaying encrypted parts in the thread pager still works on the feature branch. Especially decrypting something by pressing <code>z</code>or <code>Z</code>. If that is what the code does ;) I have never used this feature, just seen the code.</li> </ul> </blockquote> <p>Decrypting works. I will test more when closer to merging.</p> <p>The <code>MAIL_</code> prefix is fine by me. I was thinking about <code>MESSAGE_</code>, but <code>MAIL_</code> is shorter :)</p> <p>There's an initial blank line in the pager on the compose screen. This doesn't appear in the actual message file.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/seifferth"><img src="https://avatars.githubusercontent.com/u/43514227?v=4" />seifferth</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <blockquote> <p>Hmm, I quite like <code>s</code> for subject and <code>S</code> for sign. Perhaps I'm just used to it. Is save part necessary on the compose screen?</p> </blockquote> <p>Well, not exposing <em>save part</em> seems a bit like cutting corners to me. As one can select part headers in both thread pager and compose now, I would find it rather counterintuitive to have different actions associated with those headers.</p> <p>Maybe we can solve it if we don't <code>s</code>ave any parts at all? I would settle for the ability to <code>w</code>rite them to disk ;) <code>w</code> is not used yet in either thread pager or compose, so we could use it consistently across both.</p> <blockquote> <p>There's an initial blank line in the pager on the compose screen. This doesn't appear in the actual message file.</p> </blockquote> <p>That's true. I wrapped the composed message in a <code>part</code> type in order to get multipart functionality. In order to keep things consistent in the code, I also wrapped plain text messages in a part. Now depending on whether the filter is used or not, this plain part will either be wrapped in another multipart/alternative part or displayed directly. As it happens, there is a blank line just above every message part that is displayed by bower ;) So that's where the blank line comes from.</p> <p>It would be possible to somehow strip that blank line, I suppose. I left it in there for now, however, because I am actually not quite sure yet as to whether this change is a bug or a feature. The blank line is sort of unnecessary for plain text only emails, I guess, but I quite like it for multipart ones, as it separates the <code>[-- text/plain --]</code> part header from the pager boundary a bit. Now if we strip the blank line in one but not the other, pressing <code>H</code> would move the content up or down two lines, rather than just one, which I would find less appealing visually. Not having the blank line would also make the display of plain text email indistinguishable from the display of multipart email scrolled down by two lines. In a way that blank line acts as a small visual cue that there is no multipart header between the beginning of the message and the first line and that I didn't just scroll that header off the screen.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/wangp"><img src="https://avatars.githubusercontent.com/u/936637?v=4" />wangp</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <blockquote> <p>Well, not exposing <em>save part</em> seems a bit like cutting corners to me. </p> </blockquote> <p>I don't think so. The message preview on the compose screen was never meant to replicate the functionality of the thread pager, partly because of the inevitable key conflicts. It's fine that you've exposed the open URL/parts functionality on the compose screen but I wouldn't miss it if it wasn't there.</p> <p>On a related note, I noticed that quoted text gets folded now in the message preview. I don't think it was a deliberate change, and I prefer the old behaviour of not hiding anything in the message preview. (You don't need to change it right now.)</p> <blockquote> <p>Maybe we can solve it if we don't <code>s</code>ave any parts at all? I would settle for the ability to <code>w</code>rite them to disk ;) <code>w</code> is not used yet in either thread pager or compose, so we could use it consistently across both.</p> </blockquote> <p>You can add <code>w</code> on both screens if you like, but please leave <code>s</code> for save parts on the thread pager.</p> <blockquote> <blockquote> <p>There's an initial blank line in the pager on the compose screen. This doesn't appear in the actual message file.</p> </blockquote> <p>That's true. I wrapped the composed message in a <code>part</code> type in order to get multipart functionality. In order to keep things consistent in the code, I also wrapped plain text messages in a part. Now depending on whether the filter is used or not, this plain part will either be wrapped in another multipart/alternative part or displayed directly. As it happens, there is a blank line just above every message part that is displayed by bower ;) So that's where the blank line comes from.</p> </blockquote> <p>I see. It's a matter of passing <code>yes</code> for the "LastBlank0" argument to <code>flatten</code> for this case, indicating that there is already a visual separator before the part, no need for another separator.</p> <blockquote> <p>It would be possible to somehow strip that blank line, I suppose. I left it in there for now, however, because I am actually not quite sure yet as to whether this change is a bug or a feature. The blank line is sort of unnecessary for plain text only emails, I guess, but I quite like it for multipart ones, as it separates the <code>[-- text/plain --]</code> part header from the pager boundary a bit. </p> </blockquote> <p>It does look a bit noisy there, but the blank line is too misleading IMHO.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/seifferth"><img src="https://avatars.githubusercontent.com/u/43514227?v=4" />seifferth</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <blockquote> <p>The message preview on the compose screen was never meant to replicate the functionality of the thread pager, [...] It's fine that you've exposed the open URL/parts functionality on the compose screen but I wouldn't miss it if it wasn't there.</p> </blockquote> <p>I see why that wasn't necessary before. Once I could compose multipart/alternative emails, however, I greatly missed the ease with which I can inspect them in thread pager. Especially when I am experimenting with the filter command, open part becomes incredibly valuable. If I didn't have that, I would need to send the email before I could see what the filter output looks like. That really didn't seem right. (There are many other ways of getting some equivalent filter output, of course, but none of those feels particularly convenient and none gives me any guarantees that I am seeing the exact same string bower received.) Similarly, if I embed css into the html part, I really want to have a convenient way to preview that in firefox or some other graphical tool. I guess it comes down to how much one uses multipart/alternative.</p> <blockquote> <p>partly because of the inevitable key conflicts</p> </blockquote> <p>Well, that could be seen as a reason to keep things separate. Or it could be seen as a reason to rethink the strategy used for assigning keybindings ;) But I guess that's a matter of taste, somehow.</p> <blockquote> <p>On a related note, I noticed that quoted text gets folded now in the message preview. I don't think it was a deliberate change [...]</p> </blockquote> <p>You're right, of course. Just as the initial blank line, this was no deliberate change either. It came with reusing thread pager functionality in compose. I did notice both changes myself, but I am still undecided about how to handle them. On the one hand, I completely understand why you designed compose the way you did (no folding, no initial blank line). On the other hand, I also think that consistency between compose and thread pager is, generally speaking, a very desirable thing. This is why I went for wrapping plain text only email in <code>part</code> as well. I could have left everything as it was for plain text only (although with slightly more code duplication, I suppose). I didn't do that, however, because I wanted to achieve a higher level of consistency between plain text only and multipart/alternative.</p> <blockquote> <p>It does look a bit noisy there, but the blank line is too misleading IMHO.</p> </blockquote> <p>Actually, I don't find the blank line particularly misleading myself. Once I had noticed that it was there, it felt quite natural to me within seconds. I suppose this is why I care more about reducing noise here.</p> <p>Our different experiences might also be related to how much use we make of the multipart/alternative feature. Maybe you should set <code>use_alt_html_filter = default</code> in your config for a while and see how things feel.</p> <blockquote> <p>You can add <code>w</code> on both screens if you like, but please leave <code>s</code> for save parts on the thread pager.</p> </blockquote> <p>Agreed. I'll just make <code>s</code> an alias for <code>w</code> in thread pager, if that's fine with you.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/seifferth"><img src="https://avatars.githubusercontent.com/u/43514227?v=4" />seifferth</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <blockquote> <p>It does look a bit noisy there, but the blank line is too misleading IMHO.</p> </blockquote> <p>OK, the initial blank line just gave me a wtf moment, so I'll have to agree with you. I made a commit that removes it for plain text only while retaining it for multipart/alternative. I hope that's fine.</p> <p>I guess that leaves quote folding as the last open question with this issue, right?</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/wangp"><img src="https://avatars.githubusercontent.com/u/936637?v=4" />wangp</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Yeah, I'll look at the changes again on the weekend and merge.</p> <p>I'm willing to leave the initial blank line and the quote folding as is, for now anyway :)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/wangp"><img src="https://avatars.githubusercontent.com/u/936637?v=4" />wangp</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>I've merged your branch and made a few fixes. Thanks again.</p> <p>There's a couple of issues that I don't know when I'll get around to. Feel free to tackle them if you want :)</p> <ul> <li>when adding the HTML part manually, it would make sense to switch to the HTML alternative part automatically</li> <li>resizing the compose screen causes the message preview to be regenerated from scratch, which is pretty irritating. This also affects users not using HTML alternatives.</li> </ul> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/seifferth"><img src="https://avatars.githubusercontent.com/u/43514227?v=4" />seifferth</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Here's a first try at the issues: <a href="https://github.com/wangp/bower/pull/75">https://github.com/wangp/bower/pull/75</a>. Github didn't resolve the link to this ticket, so that's why I add the reference manually.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/seifferth"><img src="https://avatars.githubusercontent.com/u/43514227?v=4" />seifferth</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>I believe that, except for the part of conditionally switching to the HTML alternative, this feature is pretty much done. I have been using it extensively for the last months and am actually not even bothered by the manual switching. Pressing <code>vz</code> is easy enough and more often than not, I'm using <code>vzo</code> anyway, since double-checking my css styles work is easier in firefox than it is in w3m ;) So whether to use <code>vzo</code> or simply <code>vo</code> is not much of a difference. The current behaviour also has the advantage of being more consistent, as it works regardless of whether or not I had opened the message before, so it's easier on the muscle memory.</p> <p>Are there still open issues to be discussed, or could this ticket be closed?</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/wangp"><img src="https://avatars.githubusercontent.com/u/936637?v=4" />wangp</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Let's close it :)</p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>