When Preferences->Compose->Wrapping->Auto wrapping is switched on the wrapping is quite inconsistent. Lines are wrapped at different lengths (e.g., yesterday I had two lines that were equally long, yet one of them wrapped and the other didn't), and unwrapping works sometimes but sometimes doesn't. Also, it never works when pressing backspace or del (e.g., when pressing BS at the beginning of the next line, or when shortening a line that is already too long). All in all it seems to behave quite randomly, seldom in a useful way but often making messages turn out quite odd. (<-e.g. like this) I (and probably many others) expect auto wrapping/unwrapping to operate exactly like when auto-wrapping is switched off, except that the wrapping would be at the specified wrap column instead of at the edge of the text field.
Now you have two options. Either you switch on "Auto wrapping", which makes messages quite tedious to edit, or you switch off "Auto wrapping", which doesn't wrap outgoing messages at all. Neither option is good.
Oh, and "Preferences->Compose->Wrapping->Wrap pasted text" doesn't work at all. Neither does "Edit->Special paste->wrapped" in the Compose window. (I'm not sure if this is because of the same thing as these other wrapping problems, or if one should make a separate bug report for it.)
And apparently "Edit->Special paste->wrapped" in the Compose window doesn't work either. I just had to run it 4 times before it had wrapped all long lines.
But at least it does it automatically! Anyway, comment #2 and comment #3 work for me. The strangeness with some wrappings happen to my wife from time to time, but so far it's completely unreproduceable when scrutinized. You may have better luck than me. I don't touch wrapping code unless I trigger a bug myself :)
(In reply to comment #4) > But at least it does it automatically! Not when you use Delete or Backspace. And it never unwraps, which makes editing messages very tedious. E.g. if pressing "X" once makes the current line wrap, then pressing Backspace (and thus removing the "X") should make it unwrap accordingly. Why not make it behave like pretty much every other textbox (including itself when auto-wrapping is turned off) on the planet behaves? It really shouldn't be that hard. Here's a simple fix that's quick, easy and good enough for 95% of all people (my conservative estimate): make the "Compose message" window exactly "wrap margin" columns wide (and make sure to never hide the vertical scrollbar, lest the text box would become narrower when the scrollbar appears). Then the auto-wrap option would essentially become a "finalize wraps on send" option. This issue is the one reason why I absolute cannot recommend CM to others (in fact, if I were to edit a message in CM in front of my friends I'm sure they would ask me how I can stand it (to which I would answer that all other email clients for linux suck)). > Anyway, comment #2 and comment #3 work for me. Seriously?!? It's completely reproducible here. Are you 100% sure that you've switched on "Preferences->Compose->Wrapping->Wrap pasted text" AND switched OFF "Preferences->Compose->Wrapping->Auto wrapping"? And then when you paste ONE line that is, say, 120 characters, then it is wrapped at the _wrap_margin_? > The > strangeness > with > some wrappings happen to my wife from time to time, but so far it's completely > unreproduceable when scrutinized. I can't reproduce it either at the moment. Still, that doesn't mean the bug isn't there.
Well... I had the same "problem" here with unwrapping a line. If you have everything enabled for wrapping, just type a normal message in your composing-window. Then go into one line in your message and press DEL or Backspace. The lines after that line isn't unwrapped und you will see a new break in your message. That is a little bit ugly because I need to unwrap it by myself.
(In reply to comment #5) > Why not make it behave like pretty much every other textbox (including itself > when auto-wrapping is turned off) on the planet behaves? It really shouldn't be > that hard. Here's a simple fix that's quick, easy and good enough for 95% of > all people (my conservative estimate): make the "Compose message" window > exactly "wrap margin" columns wide (and make sure to never hide the vertical > scrollbar, lest the text box would become narrower when the scrollbar appears). Ah right, good idea! So simple I wonder how I didn't think of it before! Can you help me patch the following piece of code? I'm so stupid that I have no idea how to do this very quick and simple fix! static gboolean compose_beautify_paragraph(Compose *compose, GtkTextIter *par_iter, gboolean force) { GtkTextView *text = GTK_TEXT_VIEW(compose->text); GtkTextBuffer *buffer; GtkTextIter iter, break_pos, end_of_line; gchar *quote_str = NULL; gint quote_len; gboolean wrap_quote = prefs_common.linewrap_quote; gboolean prev_autowrap = compose->autowrap; gint startq_offset = -1, noq_offset = -1; gint uri_start = -1, uri_stop = -1; gint nouri_start = -1, nouri_stop = -1; gint num_blocks = 0; gint quotelevel = -1; gboolean modified = force; gboolean removed = FALSE; gboolean modified_before_remove = FALSE; gint lines = 0; gboolean start = TRUE; if (force) { modified = TRUE; } if (compose->draft_timeout_tag == -2) { modified = TRUE; } compose->autowrap = FALSE; buffer = gtk_text_view_get_buffer(text); undo_wrapping(compose->undostruct, TRUE); if (par_iter) { iter = *par_iter; } else { GtkTextMark *mark; mark = gtk_text_buffer_get_insert(buffer); gtk_text_buffer_get_iter_at_mark(buffer, &iter, mark); } if (compose->draft_timeout_tag == -2) { if (gtk_text_iter_ends_line(&iter)) { while (gtk_text_iter_ends_line(&iter) && gtk_text_iter_forward_line(&iter)) ; } else { while (gtk_text_iter_backward_line(&iter)) { if (gtk_text_iter_ends_line(&iter)) { gtk_text_iter_forward_line(&iter); break; } } } } else { /* move to line start */ gtk_text_iter_set_line_offset(&iter, 0); } /* go until paragraph end (empty line) */ while (start || !gtk_text_iter_ends_line(&iter)) { gchar *scanpos = NULL; /* parse table - in order of priority */ struct table { const gchar *needle; /* token */ /* token search function */ gchar *(*search) (const gchar *haystack, const gchar *needle); /* part parsing function */ gboolean (*parse) (const gchar *start, const gchar *scanpos, const gchar **bp_, const gchar **ep_, gboolean hdr); /* part to URI function */ gchar *(*build_uri) (const gchar *bp, const gchar *ep); }; static struct table parser[] = { {"http://", strcasestr, get_uri_part, make_uri_string}, {"https://", strcasestr, get_uri_part, make_uri_string}, {"ftp://", strcasestr, get_uri_part, make_uri_string}, {"sftp://", strcasestr, get_uri_part, make_uri_string}, {"www.", strcasestr, get_uri_part, make_http_string}, {"mailto:", strcasestr, get_uri_part, make_uri_string}, {"@", strcasestr, get_email_part, make_email_string} }; const gint PARSE_ELEMS = sizeof parser / sizeof parser[0]; gint last_index = PARSE_ELEMS; gint n; gchar *o_walk = NULL, *walk = NULL, *bp = NULL, *ep = NULL; gint walk_pos; start = FALSE; if (!prev_autowrap && num_blocks == 0) { num_blocks++; g_signal_handlers_block_by_func(G_OBJECT(buffer), G_CALLBACK(text_inserted), compose); } if (gtk_text_iter_has_tag(&iter, compose->no_wrap_tag) && !force) goto colorize; uri_start = uri_stop = -1; quote_len = 0; quote_str = compose_get_quote_str(buffer, &iter, "e_len); if (quote_str) { debug_print("compose_beautify_paragraph(): quote_str = '%s'\n", quote_str); if (startq_offset == -1) startq_offset = gtk_text_iter_get_offset(&iter); quotelevel = get_quote_level(quote_str, prefs_common.quote_chars); if (quotelevel > 2) { /* recycle colors */ if (prefs_common.recycle_quote_colors) quotelevel %= 3; else quotelevel = 2; } if (!wrap_quote) { goto colorize; } } else { if (startq_offset == -1) noq_offset = gtk_text_iter_get_offset(&iter); quotelevel = -1; } if (prev_autowrap == FALSE && !force && !wrap_quote) { goto colorize; } if (gtk_text_iter_ends_line(&iter)) { goto colorize; } else if (compose_get_line_break_pos(buffer, &iter, &break_pos, prefs_common.linewrap_len, quote_len)) { GtkTextIter prev, next, cur; if (prev_autowrap != FALSE || force) { compose->automatic_break = TRUE; modified = TRUE; gtk_text_buffer_insert(buffer, &break_pos, "\n", 1); compose->automatic_break = FALSE; } else if (quote_str && wrap_quote) { compose->automatic_break = TRUE; modified = TRUE; gtk_text_buffer_insert(buffer, &break_pos, "\n", 1); compose->automatic_break = FALSE; } else goto colorize; /* remove trailing spaces */ cur = break_pos; gtk_text_iter_backward_char(&cur); prev = next = cur; while (!gtk_text_iter_starts_line(&cur)) { gunichar wc; gtk_text_iter_backward_char(&cur); wc = gtk_text_iter_get_char(&cur); if (!g_unichar_isspace(wc)) break; prev = cur; } if (!gtk_text_iter_equal(&prev, &next)) { gtk_text_buffer_delete(buffer, &prev, &next); break_pos = next; gtk_text_iter_forward_char(&break_pos); } if (quote_str) gtk_text_buffer_insert(buffer, &break_pos, quote_str, -1); iter = break_pos; modified |= compose_join_next_line(compose, buffer, &iter, quote_str); /* move iter to current line start */ gtk_text_iter_set_line_offset(&iter, 0); if (quote_str) { g_free(quote_str); quote_str = NULL; } continue; } else { /* move iter to next line start */ iter = break_pos; lines++; } colorize: if (!prev_autowrap && num_blocks > 0) { num_blocks--; g_signal_handlers_unblock_by_func(G_OBJECT(buffer), G_CALLBACK(text_inserted), compose); } end_of_line = iter; while (!gtk_text_iter_ends_line(&end_of_line)) { gtk_text_iter_forward_char(&end_of_line); } o_walk = walk = gtk_text_buffer_get_text(buffer, &iter, &end_of_line, FALSE); nouri_start = gtk_text_iter_get_offset(&iter); nouri_stop = gtk_text_iter_get_offset(&end_of_line); walk_pos = gtk_text_iter_get_offset(&iter); /* FIXME: this looks phony. scanning for anything in the parse table */ for (n = 0; n < PARSE_ELEMS; n++) { gchar *tmp; tmp = parser[n].search(walk, parser[n].needle); if (tmp) { if (scanpos == NULL || tmp < scanpos) { scanpos = tmp; last_index = n; } } } bp = ep = 0; if (scanpos) { /* check if URI can be parsed */ if (parser[last_index].parse(walk, scanpos, (const gchar **)&bp, (const gchar **)&ep, FALSE) && (size_t) (ep - bp - 1) > strlen(parser[last_index].needle)) { walk = ep; } else walk = scanpos + strlen(parser[last_index].needle); } if (bp && ep) { uri_start = walk_pos + (bp - o_walk); uri_stop = walk_pos + (ep - o_walk); } g_free(o_walk); o_walk = NULL; gtk_text_iter_forward_line(&iter); g_free(quote_str); quote_str = NULL; if (startq_offset != -1) { GtkTextIter startquote, endquote; gtk_text_buffer_get_iter_at_offset( buffer, &startquote, startq_offset); endquote = iter; switch (quotelevel) { case 0: if (!gtk_text_iter_has_tag(&startquote, compose->quote0_tag) || !gtk_text_iter_has_tag(&end_of_line, compose->quote0_tag)) { gtk_text_buffer_apply_tag_by_name( buffer, "quote0", &startquote, &endquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote1", &startquote, &endquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote2", &startquote, &endquote); modified = TRUE; } break; case 1: if (!gtk_text_iter_has_tag(&startquote, compose->quote1_tag) || !gtk_text_iter_has_tag(&end_of_line, compose->quote1_tag)) { gtk_text_buffer_apply_tag_by_name( buffer, "quote1", &startquote, &endquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote0", &startquote, &endquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote2", &startquote, &endquote); modified = TRUE; } break; case 2: if (!gtk_text_iter_has_tag(&startquote, compose->quote2_tag) || !gtk_text_iter_has_tag(&end_of_line, compose->quote2_tag)) { gtk_text_buffer_apply_tag_by_name( buffer, "quote2", &startquote, &endquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote0", &startquote, &endquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote1", &startquote, &endquote); modified = TRUE; } break; } startq_offset = -1; } else if (noq_offset != -1) { GtkTextIter startnoquote, endnoquote; gtk_text_buffer_get_iter_at_offset( buffer, &startnoquote, noq_offset); endnoquote = iter; if ((gtk_text_iter_has_tag(&startnoquote, compose->quote0_tag) && gtk_text_iter_has_tag(&end_of_line, compose->quote0_tag)) || (gtk_text_iter_has_tag(&startnoquote, compose->quote1_tag) && gtk_text_iter_has_tag(&end_of_line, compose->quote1_tag)) || (gtk_text_iter_has_tag(&startnoquote, compose->quote2_tag) && gtk_text_iter_has_tag(&end_of_line, compose->quote2_tag))) { gtk_text_buffer_remove_tag_by_name( buffer, "quote0", &startnoquote, &endnoquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote1", &startnoquote, &endnoquote); gtk_text_buffer_remove_tag_by_name( buffer, "quote2", &startnoquote, &endnoquote); modified = TRUE; } noq_offset = -1; } if (uri_start != nouri_start && uri_stop != nouri_stop) { GtkTextIter nouri_start_iter, nouri_end_iter; gtk_text_buffer_get_iter_at_offset( buffer, &nouri_start_iter, nouri_start); gtk_text_buffer_get_iter_at_offset( buffer, &nouri_end_iter, nouri_stop); if (gtk_text_iter_has_tag(&nouri_start_iter, compose->uri_tag) && gtk_text_iter_has_tag(&nouri_end_iter, compose->uri_tag)) { gtk_text_buffer_remove_tag_by_name( buffer, "link", &nouri_start_iter, &nouri_end_iter); modified_before_remove = modified; modified = TRUE; removed = TRUE; } } if (uri_start > 0 && uri_stop > 0) { GtkTextIter uri_start_iter, uri_end_iter, back; gtk_text_buffer_get_iter_at_offset( buffer, &uri_start_iter, uri_start); gtk_text_buffer_get_iter_at_offset( buffer, &uri_end_iter, uri_stop); back = uri_end_iter; gtk_text_iter_backward_char(&back); if (!gtk_text_iter_has_tag(&uri_start_iter, compose->uri_tag) || !gtk_text_iter_has_tag(&back, compose->uri_tag)) { gtk_text_buffer_apply_tag_by_name( buffer, "link", &uri_start_iter, &uri_end_iter); modified = TRUE; if (removed && !modified_before_remove) { modified = FALSE; } } } if (!modified) { debug_print("not modified, out after %d lines\n", lines); goto end; } } debug_print("modified, out after %d lines\n", lines); end: if (par_iter) *par_iter = iter; undo_wrapping(compose->undostruct, FALSE); compose->autowrap = prev_autowrap; return modified; }
(In reply to comment #7) > Ah right, good idea! So simple I wonder how I didn't think of it before! Without the irony now. The subliminal message is: *do* *not* *ever* give me technical advice when you can't back it up with technical data -- such as a patch. Do you seriously think we never thought about using GTK's wrapping ? Do you really think we then dismissed it with no reason? Here are a few: - Gtk text wrapping is done at the edge of the widget, no way to specify another place. - Gtk text wrapping is global. Either it's everywhere, either it's nowhere. No more disabling wrap for one or two paragraph. - Gtk text wrapping is 100% soft. Caller has no way of knowing where GTK wrapped. - Gtk text wrapping does not rewrap quotes (nice Outlook touch to have quote chars >> in the middle of lines, isn't it)
(In reply to comment #7) > Can you help me patch the following piece of code? Umm... that doesn't seem to actually be the code that creates the "Compose message" window. (In reply to comment #8) > - Gtk text wrapping is done at the edge of the widget, no way to > specify another place. That was why I suggested to put the edge of the widget at the wrap column. > - Gtk text wrapping is global. Either it's everywhere, either it's > nowhere. No more disabling wrap for one or two paragraph. I didn't know this mattered. Good point. > - Gtk text wrapping is 100% soft. Caller has no way of knowing where GTK > wrapped. If the caller really can't find out where GTK wrapped the text after it wrapped the text then that is indeed a huge problem. > - Gtk text wrapping does not rewrap quotes That's a very good point, and because of this my earlier statement about "good enough for 95% of all people" is clearly incorrect, since I bet that a MUCH more than 5% of all people want the quote-wrapping. > *do* *not* *ever* give me technical advice when you can't back it up > with technical data Umm.. if you consider suggestions advice then that's a very silly attitude. Does it mean a feature request *must* *never* *be* *written* unless the person suggesting it knows how to actually implement it himself/herself? IMHO comment #8 is for the most parts very good (especially the "Here are a few" list, which is a very good answer that my idea doesn't work), but some of it, as well as comment #7, seems to be some kind of knee-jerk reaction with more emotion than information. I already had a look at the code yesterday, but didn't understand much of it. Would it help if I make a small "standalone" wrapping algorithm (e.g. in java)? If so, which events should that algorithm support? Probably at least delete(from,to) and insert(text,pos,wrap), but is something else also needed?
(In reply to comment #9) > Umm.. if you consider suggestions advice then that's a very silly attitude. > Does it mean a feature request *must* *never* *be* *written* unless the person > suggesting it knows how to actually implement it himself/herself? Did you ever consider how generally people ask for feature requests? I think you miss a simple point (complicate for you) Here do not works in the way: "cmon, this sucks do coding cause i'm bored of it" This even told by your boss would sound silly. Try to reconsider what is silly for you.
(In reply to comment #9) > (In reply to comment #7) > > Can you help me patch the following piece of code? > > Umm... that doesn't seem to actually be the code that creates the "Compose > message" window. It's the code that does the wrapping (and colouring). > (In reply to comment #8) > > - Gtk text wrapping is done at the edge of the widget, no way to > > specify another place. > > That was why I suggested to put the edge of the widget at the wrap column. Yes. But that'd only work with lots of constraining of the widget (or fixing the compose's window width), and only with fixed fonts. > > - Gtk text wrapping is 100% soft. Caller has no way of knowing where GTK > > wrapped. > > If the caller really can't find out where GTK wrapped the text after it > wrapped the text then that is indeed a huge problem. It's because this information is stored nowhere, it's really a soft wrap. The contents of a GtkTextBuffer containing a very long line, spanning N lines on display, will contain no \n. Maybe the information is somewhere at the Pango or Cairo level, but that makes it really hard to use. > > *do* *not* *ever* give me technical advice when you can't back it up > > with technical data > > Umm.. if you consider suggestions advice then that's a very silly attitude. > Does it mean a feature request *must* *never* *be* *written* unless the person > suggesting it knows how to actually implement it himself/herself? No, I mean I don't like implementation ideas that are are not backed up by studying feasability, drawbacks and feature regression. :) > IMHO comment #8 is for the most parts very good (especially the "Here are a > few" list, which is a very good answer that my idea doesn't work), but some of > it, as well as comment #7, seems to be some kind of knee-jerk reaction with > more emotion than information. Right, I was annoyed, by a mix of external circumstances and this bug report. Sorry about that :) > I already had a look at the code yesterday, but didn't understand much of it. > Would it help if I make a small "standalone" wrapping algorithm (e.g. in > java)? > If so, which events should that algorithm support? Probably at least > delete(from,to) and insert(text,pos,wrap), but is something else also needed? The best would be a little app that actually lets one edit text using a GtkTextView. The hard part isn't really the algorithm, but rather the performance and corner cases. To have it (feature-wise) on par with the existing, it needs to: - be able to insert large blocks of text (wrapped or unwrapped), recognise URIs in it and correctly move quotation symbols - atomically from undo's point of view - be able to handle text insertions (char by char, typically when typing), rewrapping following lines of the same paragraph on the fly, recognising word boundaries for URI coloration and spell checker, And to be complete, it'd need to: - handle text deletions (char by char or not), rewrapping following lines of the same paragraph.
I'd just like to chime in and say that I haven't had any issue with the current wrapping method. ;)
(In reply to comment #10) > Did you ever consider how generally people ask for feature requests? > I think you miss a simple point (complicate for you) > Here do not works in the way: "cmon, this sucks do coding cause i'm > bored of it" I don't know what you're smoking (not do I understand what you're trying to say, because of the many errors in your language), but I'm fairly certain that I've said nothing of that kind. In this bug report I first wrote what's wrong and then what I expected. Then in the comment you replied to I implied to offer to create the algorithm myself. (In reply to comment #11) > (In reply to comment #9) > > (In reply to comment #7) > > > Can you help me patch the following piece of code? > > > > Umm... that doesn't seem to actually be the code that creates the > > "Compose message" window. > > It's the code that does the wrapping (and colouring). Then it's obviously not the correct code for my "quick and easy fix", but you've now shown that that fix wouldn't work anyway, so none of it matters anymore. > The hard part isn't really the algorithm, but rather the performance > and corner cases. To have it (feature-wise) on par with the existing, > it needs to: > > - be able to insert large blocks of text (wrapped or unwrapped), I don't see any reason to separate inserting a lot of text and inserting a little bit of text (such as a character). > recognise URIs in it IMO that's not the job of the wrapping algorithm. I supposed there already is some URI-detection code, so why not use that code? > and correctly move quotation symbols - atomically from undo's point > of view Yes, of course. What does the undo API look like? > - be able to handle text insertions (char by char, typically when > typing), rewrapping following lines of the same paragraph on the fly, Yup. > recognising word boundaries for URI coloration and spell checker, This again seems to be a separate issue. I don't think it's a good idea to make a gazillion different things in one piece of code. Even though you manage to do it the code becomes unmaintainable. E.g., IMHO it's better to have a Boolean isWrappable(StructuredString text, Offset position) and a List<Pair<Offset,Offset>> findURLs(StructuredString text, Offset from, Offset to) or somesuch. > And to be complete, it'd need to: > - handle text deletions (char by char or not), rewrapping following > lines of the same paragraph. Naturally.
(In reply to comment #13) > I don't know what you're smoking (not do I understand what you're trying to > say, because of the many errors in your language), but I'm fairly certain that > I've said nothing of that kind. In this bug report I first wrote what's wrong > and then what I expected. Then in the comment you replied to I implied to offer > to create the algorithm myself. Don't worry I smoke good stuff you faint face. If you really want to do something useful, stop your bullshits with Java and algorithms and write a patch that fixes the issue. Ehmm.. in C, that's the language of Claws Mail, not Java.
(In reply to comment #13) > I don't see any reason to separate inserting a lot of text and inserting a > little bit of text (such as a character). Undo update (we wouldn't want Ctrl-Z to revert char by char ;) > > recognise URIs in it > > IMO that's not the job of the wrapping algorithm. I supposed there already is > some URI-detection code, so why not use that code? Yes, that's what I meant. Use the URI-detection code in insert-text handler. > > and correctly move quotation symbols - atomically from undo's point > > of view > > Yes, of course. What does the undo API look like? I don't really remember, but it's quite easy (see undo.[ch]) > > recognising word boundaries for URI coloration and spell checker, > > This again seems to be a separate issue. I don't think it's a good idea to > make a gazillion different things in one piece of code. Even though you manage > to do it the code becomes unmaintainable. Of course, these various bits could be in separate functions, but still called by insert_text callback. We want to avoid parsing the text N times for performance reasons -- and it does count!