Implement Ibis.formatting_parse

Review Request #3940 — Created March 29, 2025 and submitted

Information

ibis/ibis
default

Reviewers

This will parse IRC formating into a Pango.AttrList per the rules defined in
https://modern.ircdocs.horse/formatting.

Ran the tests under valgrind and called in the turtles.

Summary ID
Fix a few issues with formatting
Namely this fixes background colors incorrectly getting unset when just the foreground color is changed.
242cff706288cdfd6ad76d11861defb7853508bd
Description From Last Updated

I think this should be of size 3, to have space for terminating 0 character in case of length is …

ivanhoeivanhoe

I'm confused. Shouldn't this start the attribute here?

ivanhoeivanhoe

You only appear to have 0-98 in the palette, but buffer could be 99, which would read out-of-bounds.

QuLogicQuLogic

Why is this unsetting the background? The spec says: If only the foreground color is set, the background color stays …

QuLogicQuLogic

Similar here, since it follows the same rules.

QuLogicQuLogic

This is already handled in ibis_formatting_parse, so can be dropped.

QuLogicQuLogic

I don't quite understand this comment; what is "all color"? Anyway, background is reset if neither fg nor bg are …

QuLogicQuLogic
There are no open issues
ivanhoe
  1. 
      
  2. ibis/ibisformatting.c (Diff revision 1)
     
     
    Show all issues

    I think this should be of size 3, to have space for terminating 0 character in case of length is 2.

  3. ibis/ibisformatting.c (Diff revision 1)
     
     
    Show all issues

    I'm confused. Shouldn't this start the attribute here?

    1. No, the color formatting is a bit weird. It can be specified as \003#, or \003## to set a foreground, but a \003 with no number will end a previous color. So say wanted just the first word to use color twelve it'd look like \00312Hello\003! and Hello would be colored by the ! would not.

      Also, a background color can only be set if a foreground color is set, you can't to do \003,15 to set the background to color 15. You'd have to do something like \00312,15.

      Also if there's just a stray color byte, it gets ignored. For example hiya!\003! is just hiya!.

  4. 
      
grim
ivanhoe
  1. Ship It!
  2. 
      
grim
QuLogic
  1. 
      
  2. ibis/ibisformatting.c (Diff revision 2)
     
     
    Show all issues

    You only appear to have 0-98 in the palette, but buffer could be 99, which would read out-of-bounds.

    1. I was suppsoed to special case this and I forgot..

  3. ibis/ibisformatting.c (Diff revision 2)
     
     
     
    Show all issues

    Why is this unsetting the background? The spec says:

    If only the foreground color is set, the background color stays the same.

    It should only be reset if neither foreground nor background are specified.

  4. ibis/ibisformatting.c (Diff revision 2)
     
     
     
    Show all issues

    Similar here, since it follows the same rules.

  5. ibis/ibisformatting.c (Diff revision 2)
     
     
     
     
     
    Show all issues

    This is already handled in ibis_formatting_parse, so can be dropped.

  6. 
      
grim
grim
QuLogic
  1. 
      
  2. ibis/ibisformatting.c (Diff revision 3)
     
     
     
    Show all issues

    I don't quite understand this comment; what is "all color"? Anyway, background is reset if neither fg nor bg are set.

    1. I think you may have missed the second copy of this comment.

  3. 
      
grim
grim
QuLogic
  1. Ship It!
  2. 
      
grim
Review request changed
Status:
Completed
Loading...