)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"51a8e2351260a92c196eea00df78d05d073359e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"cb5acf0d_e4dd740a","updated":"2023-01-26 17:26:06.000000000","message":"I\u0027m not sure if there\u0027s a more elegant/concise way to (unchecked) bitshifting, other than casting it to the bigger size, doing the calculation there, and shifting it back to u8. Any ideas?","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"ca2b1026ce6fb4a15b9d96b37bdbd261028ef281","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"6785c11a_7eebd283","updated":"2023-01-27 05:13:25.000000000","message":"Looks good.  Somehow I am totally not surprised that Nix decided it would be a great idea to use *fractional* bytes.  For no good reason.\n\nToo bad we don\u0027t have a general-purpose\n\n```\nfn bit_chunk_iterator(input_bits:\u0026[u8], bits_per_output_chunk:u8) -\u003e Iterator\u003cu32\u003e\n```\n\n... it would factor out the hairiest parts of both `encode()` and `decode()` into a single place.  Maybe I should write one.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0ce5e2cf674224f0b29b5007f1d84708ba14f1e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"54e29b56_ff646161","in_reply_to":"3687d5a2_567c1449","updated":"2023-01-27 12:17:46.000000000","message":"Ah, that explains why I had subtle differences while trying to adapt my code to it. Thanks! So I\u0027ll instead incorporate your suggestions from above.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"1f83d5f5cc86165d103fd89ac2d265f401a59d0d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3687d5a2_567c1449","in_reply_to":"53c4a6e4_b72185f1","updated":"2023-01-27 07:18:15.000000000","message":"It looks like unchecked left/right shifts are still nightly-only:\n\nhttps://github.com/rust-lang/rust/issues/85122","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0ce5e2cf674224f0b29b5007f1d84708ba14f1e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1661cb08_0f77fc94","in_reply_to":"6785c11a_7eebd283","updated":"2023-01-27 12:17:46.000000000","message":"If there was, I\u0027d be more than happy to pull this in ;-)","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"aa3c348a04dc148c474f44e6ae574f2fd61e2701","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"53c4a6e4_b72185f1","in_reply_to":"717a441c_620eb330","updated":"2023-01-27 05:15:49.000000000","message":"That is a really confusingly-named method... it doesn\u0027t do the \"overflow-discarding shift\" that we want.  Instead of discarding the value, it discards *bits from the shift amount*!","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"badbb2709633fa146c24b7d6d4f1a8e3f37931d9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"717a441c_620eb330","in_reply_to":"cb5acf0d_e4dd740a","updated":"2023-01-27 00:42:11.000000000","message":"There\u0027s https://doc.rust-lang.org/std/primitive.u8.html#method.wrapping_shl apparently, I\u0027ll rewrite this to use that.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"}],"tvix/store/src/nixbase32.rs":[{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"ca2b1026ce6fb4a15b9d96b37bdbd261028ef281","unresolved":true,"context_lines":[{"line_number":2,"context_line":"//!"},{"line_number":3,"context_line":"//! Nix uses a custom alphabet. Contrary to other implementations (RFC4648),"},{"line_number":4,"context_line":"//! encoding to \"nix base32\" doesn\u0027t use any padding, and reads in characters"},{"line_number":5,"context_line":"//! in reverse order."},{"line_number":6,"context_line":"//!"},{"line_number":7,"context_line":"//! This is also the main reason why we can\u0027t use `data_encoding::Encoding` -"},{"line_number":8,"context_line":"//! it gets things wrong if there normally would be a need for padding."}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"03b9af60_019383c2","line":5,"updated":"2023-01-27 05:13:25.000000000","message":"We should mention that `nixbase32`-encoded strings in real-life `narinfo` files can have non-integer byte lengths (e.g. 260 bits \u003d 32.5 bytes).  When I first read this I thought nix just made different choices about *how* to pad, rather than not padding at all.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0ce5e2cf674224f0b29b5007f1d84708ba14f1e4","unresolved":true,"context_lines":[{"line_number":2,"context_line":"//!"},{"line_number":3,"context_line":"//! Nix uses a custom alphabet. Contrary to other implementations (RFC4648),"},{"line_number":4,"context_line":"//! encoding to \"nix base32\" doesn\u0027t use any padding, and reads in characters"},{"line_number":5,"context_line":"//! in reverse order."},{"line_number":6,"context_line":"//!"},{"line_number":7,"context_line":"//! This is also the main reason why we can\u0027t use `data_encoding::Encoding` -"},{"line_number":8,"context_line":"//! it gets things wrong if there normally would be a need for padding."}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"08a99856_f779c27e","line":5,"in_reply_to":"03b9af60_019383c2","updated":"2023-01-27 12:17:46.000000000","message":"I\u0027m not sure I understand this comment - 32 characters have 260 bits of information.\n\nIn general, he number of bits in a *base encoded string might not align with the number of bits you intend to encode - which from my understanding one of the reasons why there\u0027s normally padding - to make it a bit more clear.\n\nBut this is not something specific to nixbase32 - it simply doesn\u0027t use padding, but it\u0027s still unambiguous (?) how much bytes are encoded, because we can count how many groups of 5 bits can fit (?)\n\nDo you have a better proposal for the comment above?","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d00486d7b71eab142aac9a687d6fdd988dda960f","unresolved":false,"context_lines":[{"line_number":2,"context_line":"//!"},{"line_number":3,"context_line":"//! Nix uses a custom alphabet. Contrary to other implementations (RFC4648),"},{"line_number":4,"context_line":"//! encoding to \"nix base32\" doesn\u0027t use any padding, and reads in characters"},{"line_number":5,"context_line":"//! in reverse order."},{"line_number":6,"context_line":"//!"},{"line_number":7,"context_line":"//! This is also the main reason why we can\u0027t use `data_encoding::Encoding` -"},{"line_number":8,"context_line":"//! it gets things wrong if there normally would be a need for padding."}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"a46bdd8a_a70383f8","line":5,"in_reply_to":"08a99856_f779c27e","updated":"2023-01-30 11:46:26.000000000","message":"Marking as resolved for now, feel free to followup.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"ca2b1026ce6fb4a15b9d96b37bdbd261028ef281","unresolved":true,"context_lines":[{"line_number":34,"context_line":"        for n in (0..\u003doutput_len - 1).rev() {"},{"line_number":35,"context_line":"            let b \u003d n * 5;"},{"line_number":36,"context_line":"            let i \u003d b / 8;"},{"line_number":37,"context_line":"            let j \u003d b % 8;"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"            let mut c \u003d input[i] \u003e\u003e j;"},{"line_number":40,"context_line":"            if i + 1 \u003c input.len() {"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"39c101a9_4f446a5e","line":37,"updated":"2023-01-27 05:13:25.000000000","message":"It would be nice if these had comments explaining that `b` is the bit offset within the entire (160-bit) input, `i` is the input byte index, and `j` is the bit offset within that input byte.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0ce5e2cf674224f0b29b5007f1d84708ba14f1e4","unresolved":true,"context_lines":[{"line_number":34,"context_line":"        for n in (0..\u003doutput_len - 1).rev() {"},{"line_number":35,"context_line":"            let b \u003d n * 5;"},{"line_number":36,"context_line":"            let i \u003d b / 8;"},{"line_number":37,"context_line":"            let j \u003d b % 8;"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"            let mut c \u003d input[i] \u003e\u003e j;"},{"line_number":40,"context_line":"            if i + 1 \u003c input.len() {"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"968036b0_9871377e","line":37,"in_reply_to":"39c101a9_4f446a5e","updated":"2023-01-27 12:17:46.000000000","message":"Added some comments, PTAL.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d00486d7b71eab142aac9a687d6fdd988dda960f","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        for n in (0..\u003doutput_len - 1).rev() {"},{"line_number":35,"context_line":"            let b \u003d n * 5;"},{"line_number":36,"context_line":"            let i \u003d b / 8;"},{"line_number":37,"context_line":"            let j \u003d b % 8;"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"            let mut c \u003d input[i] \u003e\u003e j;"},{"line_number":40,"context_line":"            if i + 1 \u003c input.len() {"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"fb8466bf_03501781","line":37,"in_reply_to":"968036b0_9871377e","updated":"2023-01-30 11:46:26.000000000","message":"Marking as resolved for now, feel free to followup.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"ca2b1026ce6fb4a15b9d96b37bdbd261028ef281","unresolved":true,"context_lines":[{"line_number":61,"context_line":"    // loop over all characters in reverse, and keep the iteration count in n."},{"line_number":62,"context_line":"    for (n, c) in input.iter().rev().enumerate() {"},{"line_number":63,"context_line":"        // find character in alphabet"},{"line_number":64,"context_line":"        match ALPHABET.iter().position(|e| e \u003d\u003d c) {"},{"line_number":65,"context_line":"            None \u003d\u003e return Err(Nixbase32DecodeError::CharacterNotInAlphabet(*c)),"},{"line_number":66,"context_line":"            Some(pos) \u003d\u003e {"},{"line_number":67,"context_line":"                let b \u003d n * 5;"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"b7bcc20a_4bbd1a0d","line":64,"updated":"2023-01-27 05:13:25.000000000","message":"Suggest replacing with\n\n```\nmatch decode_char(c) {\n```\n\nand adding\n\n```\nfn decode_char(encoded_char:\u0026u8) -\u003e Option\u003cu8\u003e {\n    Some(match encoded_char {\n        b\u00270\u0027..\u003db\u00279\u0027 \u003d\u003e encoded_char-b\u00270\u0027,\n        b\u0027a\u0027..\u003db\u0027d\u0027 \u003d\u003e encoded_char-b\u0027a\u0027+10_u8,\n        b\u0027f\u0027..\u003db\u0027n\u0027 \u003d\u003e encoded_char-b\u0027f\u0027+14_u8,\n        b\u0027p\u0027..\u003db\u0027s\u0027 \u003d\u003e encoded_char-b\u0027p\u0027+23_u8,\n        b\u0027v\u0027..\u003db\u0027z\u0027 \u003d\u003e encoded_char-b\u0027v\u0027+27_u8,\n        _ \u003d\u003e return None,\n    })\n}\n```","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d00486d7b71eab142aac9a687d6fdd988dda960f","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    // loop over all characters in reverse, and keep the iteration count in n."},{"line_number":62,"context_line":"    for (n, c) in input.iter().rev().enumerate() {"},{"line_number":63,"context_line":"        // find character in alphabet"},{"line_number":64,"context_line":"        match ALPHABET.iter().position(|e| e \u003d\u003d c) {"},{"line_number":65,"context_line":"            None \u003d\u003e return Err(Nixbase32DecodeError::CharacterNotInAlphabet(*c)),"},{"line_number":66,"context_line":"            Some(pos) \u003d\u003e {"},{"line_number":67,"context_line":"                let b \u003d n * 5;"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"462215db_18668cc1","line":64,"in_reply_to":"8df0e815_7b2b2603","updated":"2023-01-30 11:46:26.000000000","message":"Marking as resolved for now, feel free to followup.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0ce5e2cf674224f0b29b5007f1d84708ba14f1e4","unresolved":true,"context_lines":[{"line_number":61,"context_line":"    // loop over all characters in reverse, and keep the iteration count in n."},{"line_number":62,"context_line":"    for (n, c) in input.iter().rev().enumerate() {"},{"line_number":63,"context_line":"        // find character in alphabet"},{"line_number":64,"context_line":"        match ALPHABET.iter().position(|e| e \u003d\u003d c) {"},{"line_number":65,"context_line":"            None \u003d\u003e return Err(Nixbase32DecodeError::CharacterNotInAlphabet(*c)),"},{"line_number":66,"context_line":"            Some(pos) \u003d\u003e {"},{"line_number":67,"context_line":"                let b \u003d n * 5;"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"8df0e815_7b2b2603","line":64,"in_reply_to":"b7bcc20a_4bbd1a0d","updated":"2023-01-27 12:17:46.000000000","message":"Thanks! I added this function, and used the following docstring:\n\n```\n/// This maps a nixbase32-encoded character to its binary representation, which\n/// is also the index of the character in the alphabet.\n```\n\nPTAL.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"ca2b1026ce6fb4a15b9d96b37bdbd261028ef281","unresolved":true,"context_lines":[{"line_number":69,"context_line":"                let j \u003d b % 8;"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"                // OR the main pattern (output[i] |\u003d pos \u003c\u003c j)"},{"line_number":72,"context_line":"                output[i] |\u003d ((pos as u16) \u003c\u003c (j as u16)) as u8;"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"                // calculate the \"carry pattern\""},{"line_number":75,"context_line":"                // This is `pos \u003e\u003e (8-j)`, but we need to be explicit we"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"7fc46f81_5501345f","line":72,"updated":"2023-01-27 05:13:25.000000000","message":"It might be easier to read this if `pos` were something else (`val`?).  I keep seeing this and thinking that it represents our position within the input, rather than the decoding of the current character of input.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0ce5e2cf674224f0b29b5007f1d84708ba14f1e4","unresolved":true,"context_lines":[{"line_number":69,"context_line":"                let j \u003d b % 8;"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"                // OR the main pattern (output[i] |\u003d pos \u003c\u003c j)"},{"line_number":72,"context_line":"                output[i] |\u003d ((pos as u16) \u003c\u003c (j as u16)) as u8;"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"                // calculate the \"carry pattern\""},{"line_number":75,"context_line":"                // This is `pos \u003e\u003e (8-j)`, but we need to be explicit we"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"bce404bb_ebe45095","line":72,"in_reply_to":"7fc46f81_5501345f","updated":"2023-01-27 12:17:46.000000000","message":"I called this c_decoded. PTAL.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d00486d7b71eab142aac9a687d6fdd988dda960f","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                let j \u003d b % 8;"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"                // OR the main pattern (output[i] |\u003d pos \u003c\u003c j)"},{"line_number":72,"context_line":"                output[i] |\u003d ((pos as u16) \u003c\u003c (j as u16)) as u8;"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"                // calculate the \"carry pattern\""},{"line_number":75,"context_line":"                // This is `pos \u003e\u003e (8-j)`, but we need to be explicit we"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"9d6ff251_3c6b9b1f","line":72,"in_reply_to":"bce404bb_ebe45095","updated":"2023-01-30 11:46:26.000000000","message":"Marking as resolved for now, feel free to followup.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"ca2b1026ce6fb4a15b9d96b37bdbd261028ef281","unresolved":true,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"                let carry: u8 \u003d ((((pos as u16) \u003c\u003c 8 as u16) \u003e\u003e (8 - j as u16)) \u003e\u003e 8) as u8;"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"                // if we\u0027re at the end of dst…"},{"line_number":84,"context_line":"                if i \u003d\u003d output_len - 1 {"},{"line_number":85,"context_line":"                    // but have a nonzero carry, the encoding is invalid."},{"line_number":86,"context_line":"                    if carry !\u003d 0 {"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"0b1e8c40_55e88817","line":83,"updated":"2023-01-27 05:13:25.000000000","message":"This might be more readable:\n\n```\n  let val \u003d (pos as u16).rotate_left(j as u32);\n  output[i] |\u003d (val \u0026 0x00ff)       as u8;\n  let carry \u003d ((val \u0026 0xff00) \u003e\u003e 8) as u8;\n```","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0ce5e2cf674224f0b29b5007f1d84708ba14f1e4","unresolved":true,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"                let carry: u8 \u003d ((((pos as u16) \u003c\u003c 8 as u16) \u003e\u003e (8 - j as u16)) \u003e\u003e 8) as u8;"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"                // if we\u0027re at the end of dst…"},{"line_number":84,"context_line":"                if i \u003d\u003d output_len - 1 {"},{"line_number":85,"context_line":"                    // but have a nonzero carry, the encoding is invalid."},{"line_number":86,"context_line":"                    if carry !\u003d 0 {"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"d0fd1503_52b1f342","line":83,"in_reply_to":"0b1e8c40_55e88817","updated":"2023-01-27 12:17:46.000000000","message":"Indeed, thanks! I applied this change (modulo the rename of `pos` to `c_decoded`, as written above. PTAL.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d00486d7b71eab142aac9a687d6fdd988dda960f","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"                let carry: u8 \u003d ((((pos as u16) \u003c\u003c 8 as u16) \u003e\u003e (8 - j as u16)) \u003e\u003e 8) as u8;"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"                // if we\u0027re at the end of dst…"},{"line_number":84,"context_line":"                if i \u003d\u003d output_len - 1 {"},{"line_number":85,"context_line":"                    // but have a nonzero carry, the encoding is invalid."},{"line_number":86,"context_line":"                    if carry !\u003d 0 {"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"43aa25e2_82ff780b","line":83,"in_reply_to":"d0fd1503_52b1f342","updated":"2023-01-30 11:46:26.000000000","message":"Marking as resolved for now, feel free to followup.","commit_id":"cf307f104a7f44c4a363f64ee55a874f30839a49"}]}