I didn't undo my undo
Streamed
- Reviewed PR #1483 for OTP recovery codes - a safety feature for users who lose their 2FA device
- Fixed issue #1485 where edited comments showed βnullβ instead of content due to JavaScript DOM manipulation
- Fixed comment preview losing undo history by targeting only the preview div instead of replacing the whole form
- Fixed issue #1237 where short search terms (< 4 chars) werenβt working properly in the search parser
scratch
topics
otp recovery PR https://github.com/lobsters/lobsters/pull/1483
discussion on HIBP https://github.com/lobsters/lobsters/issues/1478
fix render of edited comments https://github.com/lobsters/lobsters/issues/1485
bug: preview loses edit history bc rerenders textarea
title search https://github.com/lobsters/lobsters/issues/1237
story merging ui
title
I didn't undo my undo
post-stream
Transcripts are generated with whisperx, so they mistranscribe basically every username and technical term. They're OK but not great, advice appreciated.
Recording
04:15Mostly software. A little bit of hardware. Some networking. Some system administration. Not so much knitting.
...36Alright. And I have been... ...trying to sort out some bugs. So on the last stream, I merged a comment or a PR to fix the comment alignment. And then since then, some users have noticed that when they edit comments on the site, instead of seeing the new edited comment, they see just the word null. that struck me as worth digging into there's also an open pull request from finder to look at so when i am not developing on the site i work on code based or you know just otherwise improving things on the site maintenance and So that means at any time here, you can pop up with questions about the site, the code base, why I did something, why I didn't do something. What's going on here? A little summary. I should maybe remove that.
06:05So you can throw your questions and comments into chat. Oh, I didn't turn on the stream notification. I'll go over to prod and do that now. And touch that. So now at the bottom here, if I reload, there's a little photo that says we're live.
...34a poll that showed up instantly, actually.
I would have expected that it got cached.
Hmm.
Lucky, I guess.
I'll have to think about that.
So let's start with reviewing this poll request, because that's probably the easiest.
pushcx https://github.com/lobsters/lob…
I wonder if you're here, you can say hi, otherwise.
07:06Aha. So we have this open bug 342. And it came up in the chat room last week during stream, actually. Somebody was asking to get help back into their account. I just sorted them out earlier today. And then...
...40Get my set set up. So a couple of times a year, someone asks to have two-factor auth turned off on their account because they have accidentally locked themselves out by losing the one place that they had the TOTP set up. And this has been... If we look in the comments, you will see... A couple of times a year, someone puts a comment saying, hey, I'm locked out. Because if you search lobsters in TOTP, this shows up first. Because it has all those keywords. So it's kind of become an informal support center. And otherwise, they show up in my inbox. There have been a few more that appear here on GitHub. So let's see what Hunter has done.
08:49Oh, and Irene, another moderator, has reviewed it. I know both of these have worked on security controls. This is actually really great to have some folks with some familiarity.
09:08All right, let's see what we got here. So we have a recovery code. Please update your OTP device. I don't know what that means.
10:22Yeah, let's just start a review on that one. Probably when I read the code, it'll become clear what the next thing to do is. Good. Recovery codes are...
...54A couple typos there, let's just suggest it in case you lose.
11:18One field on user, why not? At some point, it would probably make sense to start off a login model. But that's not happening today. So if we have a code. OK, so you just use it in place of a secret.
12:24Yeah.
13:02So I'll update that. If we find the editor and the form isn't updated to say that they can do something here. OK.
...33Spongebob
14:45This also opens the question up.
15:21Actually, you know, as soon as I write out this question of should I still be willing to disable 2FA, the answer is kind of obviously yes, because number one, we, well, every existing user with 2FA enabled should have that option because they're not going to have recovery codes. And then number two, some users aren't going to save their recovery codes because there isn't a step forcing them to. And honestly, number three, Lobster's account is just not so high stakes that we need to play on Ironman mode or permadeath mode, however you want to think about it, where if you lose 2FA, you're just locked out forever. We're not a bank account. We're not. National ID card. We're not voting. We're a forum. So, yeah. That's... Alright. Let's be clear.
16:49chamlis_ hi! audio stuttering seems to have returned to us
Oh, shit.
Sorry, I forgot my setup step.
I should be good in two seconds.
Thank you, Shamless.
chamlis_ yep that's back
Let me know if I'm back to normal now.
This is one of those days where I did not fully run my stream checklist.
Okay.
Thank you for the feedback.
I appreciate it.
Sorry for anybody who's listening to the recording because it's probably bad on the recording too because of when it happens.
All right.
17:25chamlis_ I'd love to know what pulseaudio bug you're working around with the visualisation
I spent some time on, what, Wednesday, Tuesday, somewhere, tinkering with the script for transcribing stuff, but that turned into tinkering with a larger project to reimplement Rake, which is the Ruby implementation of Make.
If I did that, yeah.
Oh yeah, I would love to know too.
I assume someone pointed out
Shpitzick Good afternoon gentlemen
think it might have been you but somebody said oh maybe it sounds like a buffer underrun and i think that's really plausible given the way the bug disappears if i open the visualizer because that would be pulling more frequently and solve something like that but i'm not a audio expert so presumably there are a hundred things it could be
18:23howdy new chatter welcome all right so oh what's the there's like the filtered parameters what is it called in rails I want to make sure I'm in the wrong. That's going to be annoying all stream. Let's drop out of that and jump back over to lobsters and restart Vim.
19:17So we have a parameter filter is the name of it. There we go.
...41Just grab that file name. And the terminal is fighting me. Super. Schleiser's. Burby.
20:28There's also, isn't there also, let's look at the user model. Yeah, I'm already on as JSON. OK, so as JSON explicitly lists the attributes that should be included, so it will not, the default one automatically picks up new attributes. I was just cleaning up these as JSONs because the tags endpoint was printing raw IDs of things, which is not in any way bad. It's just not what we want out of tags. We want to always refer to them by their name, not some arbitrary ID number. All right, cool. Great. What a nice feature.
21:22Where did Irene's comments go?
22:34Okay.
23:11This this is sort of the stripe style of prefixing things and. I almost wanted to suggest prefixing them with like lobsters underscore recovery underscore username but then. there's the whole. threat model of well what if you lose your recovery codes but not. Your other stuff, and so you want. It not to be super obvious what they are I don't know this format would be unique, but. At least it's not a giant pointer at the site. yeah. All right. What else was I gonna say. There was something else about this code let's comment, because I can always leave another.
24:37So it's an array. This is going to error for existing users. Yeah.
25:34Don't remember. Yeah, all right.
...52Actually, will it? I'm not sure.
26:55But I can't add that to my existing review. All right. Cool. So there's one pull request handled. And if anybody has any questions about the CIDR codebase, pop up anytime. It's especially nice when I'm kind of shifting gears between different topics. But they're welcome anytime.
27:27What has happened in discussion here? You know, I saw that there was a little
...59Treat password shots as radioactive.
28:09I don't understand.
...30chamlis_ I was thinking about that: single-iteration unsalted SHA is much easier to crack than what rails stores in the db
dpk0 greetings
i mean single iteration yeah and certainly is but we don't they must be come here where the minimize my browser how does this
I thought this gem took the raw password and compared it.
It didn't really make sense to me.
Okay, so there's something about sha-bloom.
chamlis_ so if somebody picks a good password the DB etc knows an easier-to-reverse hash for their real password from the failed query
Oh, hey, DPK, been a second.
Welcome back.
And Shamless, yeah, I think it's saying that has unpublished password is converting to sha-1.
that's not a good default message all right and then oh i get it so for the bloom filter i bet they're converting the raw password list from have i been pwned to sha1 so that all of these things are a consistent length
somebody picks a good password the database etc knows an easier reverse hash for their
Oh, I see what you mean.
Yeah, and they do a much more complex bit of anonymization for the API, the live one that I am trying to avoid using.
30:26Yeah.
...36I guess I'm all right.
31:02So they,
...15Let me explain why I kind of bounced off the has unpublished password, but maybe that was a conversation I had on social media and I should just repeat myself here.
32:46All right.
34:04chamlis_ I guess you could (and maybe the gem does?) query based on part of the hash and do the final compares in the app itself, like how the API works
this mice yeah it could i'm not i mean we say database but it's clear that
what it's doing here is not i think it's just the file is just a binary format so we take the bits per filter we have a bit field and so then we just yeah so walk hash is gonna say
Yeah, it's just doing a binary search.
chamlis_ ahh I see
So this isn't really anything I would call a database, because at the bottom of it, this is a file access, so it's all going to happen in memory for the Rails process.
There's no external software here that's getting a copy of the thing.
Yeah, it's worth checking, though.
I'm glad you asked to run it down.
I wonder if we'd have the same issue that I saw with the GeoIP database, where it was leaking.
So if we, because this is not, looks like it's opening the fileware.
Actually, that's a great question.
Where does it open it?
So walk hash takes a hash.
Self class filter count times.
This loads the whole hash into RAM, doesn't it?
The whole hash isn't the thing we're searching for.
I really don't want to make every
work or 32 megs bigger okay i understand the value of doing this in ram but that's a lot of ram i mean even 32 megs of process is expensive for this feature
37:10veqqio How many workers or processes are there in total?
I guess this... How many workers or processes?
It's in our config, but it's usually something like 15 processes, and then each has three threads, I want to say.
I would have to dig it up.
But if you look at the... Oh, and welcome back, Vecchio.
If you look at the...
There's some of it in the Puma config.
Yeah, so three, which is pretty standard for Rails now.
And then...
I want to say that might be overridden here.
Yeah, this says Puma workers 10.
Huh.
That's lower than I remember.
Yeah.
So 10 times three is 30 threads total 30 workers.
Well, so Puma calls a worker, a process calls a process, a worker, however you want to put it.
So I guess this is.
Filter count times do.
38:46So this serialized, this must be what's writing it. Yeah, so this is the code. This is half of the code I just looked for. Hold on. GitHub, you want to tell me who calls this? Nobody calls this.
39:13Prep and verify.
...34Now maybe this is the script.
40:45Okay.
41:15This is Bitfield thing. It's not something they define, right? Acquire Bitfield. What is the Ruby Bitfield gem?
...42Also by this author.
...52This is the default gem text, not very helpful. I'm trying to see.
42:19Yeah, so file bin read, there it is. That's doing what I expect of pulling the whole file into memory. So let's grab a permalink there so I can reference.
43:08I am at the limits of my Rails deployment knowledge where I would suggest a more specific code path here. Like, I am sure there is a way to do this. I just don't know what it is to actually suggest it.
...41I'm almost tempted to say that seeking around a file is actually fine because this happens once at login and seeking around a 32 megabyte file is You know, not even going to be measured in milliseconds.
44:45This is getting long. You know, I come up with these parentheticals and then I end up sorting them out.
45:41pushcx https://github.com/lobsters/lob…
All right.
I keep forgetting.
If I forget to send the issue link, please just go ahead and ping me.
I'm very happy to have folks able to follow along.
46:08chamlis_ ship the file to the client and have them do the lookup ;)
Ship the file to the client.
...16You know, I saw someone who, I saw a very sort of funny library that was a JavaScript SQLite client and the client ran on the browser, but the SQLite file was stored on the server and the client had been adapted to use range queries to basically do these file seeks against the database. So performance was not exceptional, just was mostly a very clever hack. But with SQLite, it does a lot of, you know, you read in the file metadata once that's up at the block at the start of the file. So sure, you say, I want to HTTP get bytes zero to, I don't know, 10k, whatever it is. And then you can just seek around inside the file, which is exactly what the SQLite library is doing. And so with the addition of a significant amount of latency, it was actually not a huge amount of code. All right. So I haven't even looked at this bug. And then this one is probably stuff from the last stream.
47:50Off topic, but fun Guinness thread. Oh, this is not a bug. This is a feature. So where's, let's browse the, let's just browse to it as fast as with GitHub. It's in the current view of replying comments.
48:44veqqio I was a bit suspicious of him adding a 6 year old package as a dependency (bitfield) but looked around and saw some talks of his etc. He also doesn't spam repos with such prs so. My own parenthetical of suspicion
Presumably, see Jeff Beer here does not link his Lobster's account.
49:04Vecchio, could you elaborate on suspicion? The Bitfield package is also his and was I went through it pretty fast on rubygems.org there, but I could see that it's timeline lined up with the original publishing of the has unpublished password gem.
...45veqqio Nothing to worry about
veqqio I think it's fine
veqqio I was just worried when looking at it at first
Okay.
...51You know, one quirk of... Yeah, I actually... I actually kind of slid off of his has unpublished password because I turned it up when I was looking to see if there was already a gem to handle the have I been pwned lists. And the fact that it was five years old didn't immediately put me off because I could see how such a thing could be written in a way that like the active record API there that it's going on top of hasn't changed that much. hasn't really changed at all in the last five years. So I could see how it would be good. But when I clicked around it, it's exactly the thing I just left a comment about, I didn't actually see all of the missing parts for downloading the scripts, and there wasn't any documentation of it. And then I didn't realize that he was in some way baking the Bloom filter into the gem. And even if he was, the last published gem was what? Four and a half years ago, something? So well out of date with the password lists. So that was my concern, was not so much the dependency as that. Although, you know, especially now that you mention it, I don't pin specific versions of gems in the gem file unless there is a reason for it. I just use the gem file lock for that, and I'm judicious about when I run bundle upgrade. This one I would want to pin a specific version, especially with a dependency, because... Given that it's going to have clear text access to passwords, you really want to review it every single update, read the whole diff. And so it would be nice if the BitField gem was folded in, especially because the Ruby BitField gem probably does not have a lot of reverse dependencies, right? yeah it only has this so just just fold it in because it's not big right so we see our percentages oh right but github doesn't tell me the the number of lines of code but call it what a couple dozen lines of code i just looked at the one lib file yeah great 58 Just to have one fewer moving part, I would want to, if I were Magic Wand, the maintainer of that gem, I would fold it in. Because then also you have to read the gem spec, because that's just Ruby. And there is nothing exciting here, but you have to re-review for every release.
53:15Alright, so jumping back, let's
54:36I didn't throw in the link, but so this one, this is one of those places where there is a lot of thought for community design wrapped up in this code.
But the basic gist of it is trying to make sure that if something is happening where people are flagging the heck out of each other or responding to people they've flagged, or the story is otherwise going off the rails,
like a flame war or something like that that we don't surface the replies on the replies view because we don't want this to pour fuel on the fire of a flame war and if you look at the version up in the url you can see it's number 10 so we went through quite a few iterations because the very first version of this had many fewer of them and i felt like for the first couple of
I don't remember exactly the speed of these releases.
I want to say first couple of weeks or maybe first couple of months.
It did feel like they were encouraging arguments.
Yeah.
Yeah, actually, maybe they came later than I remember.
Yeah, these are just fixes.
pushcx https://github.com/lobsters/lob…
And then these are the ones I'm thinking of that were about trying not to pour fuel on the fire.
All right.
So then next one, and watch me remember the link.
56:23Next one is about editing comments.
...36So Natty Narwhal guessed it was the refactoring of the comment bylines. But no, I'm going to bet this was that recent pull request. Because this tweaked the page structure a little bit. And selecting comments and javascript has a null if it said nil i would be real suspicious of code on the ruby side but the fact that it says null tells me it's javascript so this whole wrapped comment we find the reply form else
57:38probably the remove and prepend that I was just commenting on. So if this prepends comments subtree and that's not there, that would do it. That would be the bug.
58:09Alright. Superversar has joined, but he just seems to want to look out the window at birds. He's not hopping up on his perch. So there is not yet a cat stream. We'll see if he settles down. It's a nice day here in Chicago, so there have been a lot of birds around, so he's pretty excited about that. Okay, buddy, you don't need to bump me. Alright. So it's probably that. Let's go look at that. Get the Rails server running. So if I pull this up, and let's add a comment. and then edit it. Huh? Why is it? I wanted to. I distracted myself, I didn't. I wondered why the edit link wasn't underlined. That's. So flag isn't underlined, but that's kind of OK because it's the. Menu. Hmm. Although it's a little weird to flag your own comment. All right. So if I edit and I say v2, well, there's the bug. And if I reload, the edit actually happened. Yep. All right. So let's hop in the console. Let's hop in the debugger.
01:00:19And I had guessed it was here. So let's see how likely I am to be right.
...31Hey look, I hit my breakpoint.
...41So where is the... Yeah, comment subtree is null. So in the case of editing, it failed to find
01:01:02The wrapped comment. This is going to be a bit of a slog because we have a very complicated form that is very closely coupled to this JavaScript. And when I was reviewing that pull request a couple of times, I kept talking about that complicated coupling. And here's that complexity biting us.
...44This on the left.
01:02:03So this is actually maybe wrong. We don't know which of these two branches we went through first. though it did work because it removed that form. So let's go ahead and say.
...33And we are prepending.
...46Pull that debugger back up. I want it to look like I usually see it.
01:03:03I'm going to spend a bunch of time annoyed at the 1080p restriction on streaming.
...20Let's go ahead and run that. So we have our tree in scope. Reply form is null. So we went through the second branch here.
...48This is a little misleading because they didn't create a top level comment. They edited a comment and then comments is the right element. Comments subtree is null.
01:04:18Did we get back the comment? So we take the text and we create a P. We shove the text into the wrapped comment. Oh yeah, so this was the whole thing that Tomcat wrote a comment about, or a, comment on the pull request about where they are trying to hydrate the DOM by just shoving things in a P. But the edited comment doesn't come back. It's just the comment. It doesn't include what I think it wants to be. and closing li probably all right so let's let that fail and leave a new comment and i'll hit the breakpoint again
01:05:45yeah so the comment is coming back rendered slightly different so this fix can happen server side comments underscore subtree all right let's go up to the comment controller so when we create show
01:06:18Then we, if the preview is blank, we succeeded. No, if it's valid, we render the created comment, which calls this posted reply. Man, there's a template I haven't touched in ages. And there's the comments subtree that we expected.
...54But then I bet the edit method is just, or the update method is just rendering the comment, not this posted reply.
01:07:15Yup.
...24Yeah, there was a separate flow, I guess. Alright.
01:08:22So I'll make this do the same as the other, which is really just render the created comment.
...47OK, if we lose the tree lines. Well, I know I'm going to get that bug request.
01:09:16The default is true. So let's just keep that default. And then the user can override. And I guess instead of being render created comment, It's more of a render the comments that the user edited or created. That's not a great name.
01:10:18Guess I'm going to leave that for the moment.
...38So let's come back up here and see how much I just broke.
...54And then afterwards, I'll look at this underlying inconsistency. Let's edit the one I've already been editing. Still no. Hmm.
01:11:56Make that a little clearer. So what is this?
01:12:26That really felt like it should have worked. So let's reload and use the debugger. Did I clear that breakpoint? Did the breakpoint accidentally get cleared because I touched this function? Do I know what line number it was? No.
01:13:14That's what I wanted to search for. All right.
...24So we're paused, and comment subtree is still null, which is the behavior we saw. And the text came back, oh-ho, text came back argument error. All right, so there's my bug. that's let's jump over to the rails console because that'll have a more readable version of the error right unknown keywords show tree lines that is
01:14:15I thought I could do the new syntax there. All right, so let's resume that and edit this to say v2. And I got the same error. It must have been at the call site, not the render site, so this part was
...52What line are you complaining about?
01:15:01Unknown local. Oh, oh, because it's, so that's a, I gotta update the template. That's fine. All right, so it wasn't that at all. That part actually worked or we wouldn't have gotten that. So the local,
...25This should get passed down here, which actually already always says true. That looks fine.
...59Yeah, big cat, you got to lay on the mouse. Wrong kind, buddy.
01:16:12Hey, it looks like it worked. Yes. OK. So I'm going to run the test suite, but that is the bug. Let's go ahead and remove that breakpoint. Not too bad.
...36If that takes care of the bug, then I'll do the styling. No fails. What is the other change here? Why is there a change to user? Was I tinkering around and I left a typo? Let's fix that typo.
01:17:34And this typo that's been hanging out for a minute.
...47Let's just say no syntactic changes to user. That says I tweaked white space somewhere.
01:18:18or standard RB tweaked it, that's fine by me. Let's double check, though.
...35Nope, must have been me typoing or something. That's fine.
...47Oops, that comment number 1485. Easy enough. 1485.
01:19:09And deploy that. Great.
...38Cool, cool. All right. So with that getting deployed, I have looked at all of the open issues that have been touched. Yes. Oh, come on. Oh, no. There's one more. What is this? What is the span? Oh, yeah. This looked like some kind of bot trying to build up code or build up activity. All right.
01:20:41RoryOKane I see youβre changing the editing of comments. That reminds me of another bug with comments that I never got around to creating a GitHub Issue for. Itβs that after clicking Preview, on your comment, the undo history is lost.
So this was, they opened a pull request and then closed it the same day.
01:21:01RoryOKane This happens because the entire `textarea` element is replaced with what the server sends, instead of just changing the text inside the `textarea`. Itβs probably not a simple fix. But just mentioning in case youβre already in the mindset for it.
Oh, Rory, that's an interesting bug.
And what's happening there, having just looked at the JavaScript, is, oh, you know what's happened.
Yeah.
...33Yeah, I'm a little reluctant to fix that knowing how complicated that code path is. We would have to... Well, the preview...
...50The preview would have to not render the comment form, just render the...
01:22:02Especially with edit, because you can also preview the ones you edit, in which case you also get a rerender of all of its replies.
...19All right, let me peek at that before it totally leaves my head.
...33So we render the comment box.
...47So update has that same flow.
...56So if you click preview comment,
01:23:05We replace the form parent element, which is the whole comment.
...45RoryOKane I found some old comments I wrote when exploring this code, in `previewComment` within application.js.erb (as of an old version of Lobsters):
Oh, I'm happy to have your notes.
RoryOKane response.text().then(text => { // TODO: don't replace the whole form. Replace the parts around the form that could change as a result of clicking Preview.
Yeah.
Thank you.
RoryOKane // The parts to change might be only the `<div class="preview">` within app/views/comments/_commentbox.html.erb β if so, just extract one partial with that, then ensure the `preview` method in CommentsController (called as a result of this JSβs POST request) renders just that partial, and target some area below the comment input field in this JS.
RoryOKane // Before I implement that, I just need to trace this JSβs call that renders the whole _commentbox partial and confirm that nothing above the preview is expected to change compared to the original render of the story page and the comment box added to the page by `on("click", "a.comment_replier", β¦)` in this file after clicking the βreplyβ link.
So I can see it's going to be, where's the form here.
And then it re-renders the whole comment when what we want it to render, well, it's not even present here.
So we would have to put a placeholder element in.
01:24:20Yeah, you know, it's not even the parts around the form, because the comment comes in as an element underneath. So if I say test preview one. Come here, give me the whole, yeah. See, we're no longer even in the form element. There is a new div underneath called preview. So if I had the comment box, always include that div class equals preview. I think that would just about do it. Let's take a look at that.
01:25:17No, I guess it's a bug.
...37So I don't want to replace the form parent element here. I want to say, find the sibling. And then when I render the comment box, if we're previewing,
01:26:06We don't render that part.
...14Actually, we don't want to hit this at all. We just want to go straight to this part and just render the comment in a div called preview.
...31Yeah. So let's make sure that's always present.
...56And then this comes away. Let's make it a separate partial.
01:27:22And then actually. If the comment is there, we do want to render it.
...37Yeah, so I still want to keep this if.
...56Let's just move it down.
01:29:15That's pretty good.
...29Let's update the controller. Yeah.
...57And that looks like that. done right there. And then so this one, how did I write querySelector? I want to say it takes up parent element, right?
01:30:31Yeah, it takes a context and a selector. So what we want to say is in the context of our parent element, we want to find the preview.
01:31:02that's what's going to get replaced that might be it let's test all right so reload oh got a bug did i save it wrong missing partial stories preview oh so we're on the stories view so it wanted the all right all right So in the comment box, better.
...59I got to put the locals hash around it.
01:32:18Thinking pretty hard about that, Rails. Oh, it's the darn cache waiting to get filled. Shouldn't take too long there.
...48any time rails so this is why i don't like that with flying comments view and i'd like to get rid of it so it's filling a cache there and there it goes and it took its sweet time about filling the cache all right
So I don't actually want to have the same form.
Don't tell me you're filling the cache again.
All right, so preview two.
All right, so that worked.
And this form didn't have a preview element on it.
Let's check that this one does.
Yes.
What are we erroring?
Yeah, you're mad about that JS error.
That's fine.
And then this one doesn't, because it's getting rendered differently somehow.
No, it does.
All right, so I just have an actual bug.
Yeah.
Let's go look at the bug.
QSI typoed.
It's Q capital S for query selector.
Try that again.
All right, so I got a preview.
It's indented, but I want to say that top level 1.0 is already.
And I can Control-Z because the text area didn't get replaced.
So let's start a reply.
ASDF.
ASDF preview.
Undo.
There we go.
How's that, Rory?
RoryOKane Wow, it will be amazing to have that finally fixed.
I think I actually can't get the preview to come out.
Yeah, that'll be nice.
All right, let's look at the diff just to review.
Well, thanks for reporting the bug.
I appreciate it.
So yeah, that changed, that changed.
This got moved over here.
All right, so I expect that the build will stay green here and then I'll push this in a minute.
Thanks for telling me about it.
It's one of those things where I'm so familiar with how the code works, it didn't even register to me as a bug, because I know that it's replacing the whole form.
But I can see how that's annoying.
All right.
01:36:33All right, so Rory, I don't know that I've seen you on a lot of streams, so that deploy will take a couple of minutes in the background.
So if you don't see it instantly, if you want to be cautious, you can give it like six or seven minutes here.
When the bundle doesn't change, we do a phased restart.
So the workers get replaced one at a time in turn.
like a little bit of indeterminacy where when you reload the page, you will get the old code or the new code, depending on which worker you hit.
RoryOKane Got it, thanks. Yeah, Iβm not usually on streams.
We don't have any kind of per worker affinity based on your user, which is the sort of thing we would do in a mature web app, but we're nothing if not immature.
01:37:27Yeah. Well, it's good to have you drop by today. So with that, all right. Yeah. So let's pick back up this title.
...51pushcx https://github.com/lobsters/lob…
So here's the title search.
You know, given that both of those are comment box, I'm gonna laugh pretty hard if I just managed to break the thing that I just fixed.
01:38:21See, that is weird. What if I throw it in quotes? Huh, that shouldn't be possible that adding quotes found more things. Huh, I'm very, like I could see R being a stop word, but files and hard definitely shouldn't be. All right, so that's correct. And this, so I'm in an inconsistent state because I hit back. So it maintained my form. This is probably correct. So I would have to check. So this search should be searching the text of these files. I'm sorry, the text of these stories, which includes the actual text on the page.
01:39:32so i can i can kind of believe that they include the string or the text files are hard that would be not exactly a string match because it ignores punctuation yeah so they say things like files are hard coded okay so these are probably But this one, this first one with no quotes, that's just a failure.
01:40:22So they started to say they should search I see this looks like exactly what I want or match story texts title. Against terms dot map. In Boolean mode. Why would they need to?
...58Why would they need to explicitly call out story text title a second time?
01:41:12All right, let's take a look at this locally. Mostly, I want to see the SQL generated and then run it in the console.
...41yeah here we are so there's one to count and then one to select them so let's grab this and run this query oh man i'm gonna get i'm gonna fight word wrap
01:42:10i select like this will i avoid word wrap because i don't really don't want to manually no i have to manually fix it all right now let's struggle against read line right and i can't delete that
...44Hidden stories. Drawing the origins. All right, there we go.
01:43:14Is it because R is a stop word?
...26It's because R is a stop word. Oh, boy.
01:44:25I bet if I search for, well, if I search for title colon, yeah, there isn't an intermediate here.
01:46:25no not the search engine have the what is it called it's the search parser yeah so let's say that you
...59Thank you.
01:48:05chamlis_ iiuc mariadb says stopwords aren't discounted in boolean mode, but you might be talking about in the creation of the index? https://mariadb.com/kb/en/full-…
Okay, so this is tagged as bug.
Wasn't sure if I'd even interacted.
Oh, Shamless finding the very relevant document.
Thank you.
Let's take a look.
Okay, we're jumping over to light mode.
...28Oh, is it just the length that did it?
...38chamlis_ I wondered that but presumed you were on innodb
So what if I search for then?
...51Just to search for a four letter. It's taking long enough that I think it's actually going to find things.
01:49:08just leave that run for a minute.
...20Do not reflect in the search results unless in Boolean mode is used.
Oh, you're right.
I see.
So I must, it must just be the length, right?
And we know that because this search is actually running right now, and it gives then as an example of a stop board.
Okay.
So this is just length.
Oh, that's even better.
chamlis_ is it myiasm then?
See, I was, yeah.
So we got results.
We got every article under the sun here.
57,000.
Yeah.
01:50:22chamlis_ myisam*
Oh, I see.
Cause you're saying less than four.
We don't know.
Right.
It's less than four.
...39Yeah, it must be my ISAM.
...46Because I want to say my ISAM is the thing that is the more recent one that everybody just uses by default now. I can't even remember the last time I chose between them on purpose.
01:51:08chamlis_ I thought it was the other way around but you're probably right
And now that we've figured out its length, I want to say we might even have had another bug open about it.
...28This one's length.
...37Did it say Sonic?
01:52:30Okay.
jangomandalorian Hey @pushcx , hey chat! ππΌ
So let's just edit this because I just wrote it two seconds ago.
Hey Django, welcome.
01:53:24Shameless, would you like to be credited by name here?
...45chamlis_ I don't mind, but thank you
Don't mind.
Does that mean don't care either way or don't want?
Because I was just going to write, thanks, Shameless, here.
01:54:07chamlis_ don't mind either way, go for it!
Don't mind either way.
Yeah, well, I do really like to give credit unless people explicitly say no.
Alright, so let's go ahead and update that.
...33Let's
...50That actually is pretty straightforward, because the search parser is sort of a series of nested regular expressions. And it's easy enough to add a minimum length, right? So let's take a look at that. I don't need this.
01:55:12So if we said that a term has to be three letters, So we're going to duplicate this to short word. And then this one you have to have repeat on. I think it's repeat four. I'll write a test on both sides of it. Because I can't remember if this repeat interface includes the initial match or not. We'll check.
01:56:07Here, let's just say versus four character words.
And I'm saying four
because we were accidentally testing that with the word hard.
RoryOKane Parslet docs (https://kschiess.github.io/pars… say `.repeat(1)` matches at least once. So the number includes the initial match.
So I know that, well, yeah, but the text said less than four.
So yeah, we'll see it work versus four character words.
Hard, it doesn't parse three character words.
01:57:09So the number includes the initial match.
Ah, thanks for checking, Rory.
I was just going to write the text on either side.
Let's parse two character words.
I mean, let's just be thorough.
If there's only n equals 4, then let's go ahead and just say can't search for short terms.
And then describe short word rule.
chamlis_ later on that same page for "in boolean mode" it says "Stopwords and word minimum and maximum lengths still apply as usual." so who knows what it does; to the source code!
Rory, is there a way to say, well, actually, it searches in order.
So I shouldn't have to.
Shamless don't go to the source code.
I'll just write a couple of tests and we'll be good.
Although, or I could just pull up the search engine and search for a couple of things.
01:58:38chamlis_ haha
These should also say,
I do appreciate you searching this stuff, but especially because there's so many options.
So if I search for, well, word minimum.
01:59:13Yeah. You know, if then turned up things and R didn't, we know that that's the breakpoint. I think we're actually okay. I don't need to add more tests.
...33Let's see if that parses. No.
...47Did I not save? Oh, I called the wrong term. sp.term, that should be a short word. All right, what are the rest of my failures?
02:00:19X round parse to term, term, term.
...27I broke something with quoting. That's odd. Expected title to be able to pass. Something with quoting is broken.
...56Oh, I said as term instead of short word. Copy and paste got me twice. Although, I don't see how that could produce those bugs, and indeed it didn't.
02:01:34So this is a good time to mention that if you catch a bug before I do, you get marked as a channel VIP. Expected title.
02:02:09Yeah, this is really strange to me.
I don't understand why the titles are broken.
RoryOKane are you referencing shortword anywhere on the left?
So this worked, this worked, but this failed.
Oh, because when you quote,
I'm not referencing short word anywhere on the left.
So you can see the file name down at the bottom.
What's happening is quoted is built on term and term is no longer catching these short words.
So this is actually, this wants to be term or short word.
And I don't know how to express that off the top of my head.
But you, Rari, just linked to the Parslet docs.
So let's jump into light mode because they are.
RoryOKane section "alternation"
How do I say or?
02:03:10Section alternation. Aha.
...20All right, so let's go ahead and say term or short word, because you're allowed short words in quotes. That ought to do it. It caught my eye that this first thing was a short word, but didn't get it. Luckily, the tests were extensive enough to trip over this behavior. Otherwise, that would have been real annoying to run down in prod. All right, so I got one out of two. And then this one is fine. This one is just saying that some of these terms, one of these terms became short word. Okay, so that's fine. So let's go back down to title. All right, let's find out what line is failing. 168. Yeah, that's fine. And now this change makes sense as well. And if I had read this test failure closer, I would have caught it earlier. That's OK. Well, thanks again, Rory. All right. So we got this. Now what do we want to change? The search engine will automatically ignore them, because the way it works is by looping the rules it knows about. So we just need to fix the search view. And then here. We'll need you.
02:06:20So this is wrong. This should say. And then we'll just copy that explanation.
02:07:03We don't even need to call strip operators because it doesn't get parsed. And we're explaining that it got thrown away.
...57I'd like to use test data directly. All right, so now if I search for files are hard with no quotes, it's going to take a second because it's going to search for files in hard. This did not parse correctly. Ho, ho. Oh, it's the Do I need to update the search model?
02:08:45Yeah, so I would expect it to just fall through all this and then ignore it. So that part's correct.
02:09:11Let's check that.
...22I didn't even use the rule. That's what happened. I had to start including it here.
...35I bet that's the bug.
...52Why did this recognize this as a term, though, because I edited the definition of term to require four characters. Yeah.
02:10:17Why do I feel like that last one was indicating a subtle bug?
...29Oh, it wasn't. It was display. So I get it. So there's all of the rules, and they are in a specific order. And at the end, there's catchall, which is just if there's anything, and then catchall kind of smashes it over to be a term. And that's just for Like if you have an opening quote and no close quote, we don't know what to do with that. So it falls into catch-all where term will typically not, you know, would not otherwise match a single quote because that's not in its regex. And when I say single quote, I mean a single double quote. It does match apostrophes. All right. All right. So let's... I want to add a spec for that because we should have caught that.
02:11:55I'm going to reverse that edit because I want to see this test fail.
02:12:05And now this test will fail. That'll be, I don't know, a term or a catch-all. It just gets displayed as a term on the front end for the sake of hiding a little bit of the complexity. Good. Good. Exactly what I expected. I understand the bug. I can be confident it's fixed. Okay.
...34Great, what a nice little fix. So the first, this is like outside of academia, the second parser I've written. I think so. I'm not including using libraries for parsing options, which are kind of parser, but you're not really thinking about the parse tree there. in this way so all right nice to use some of that cs what's the bug number one two three seven
02:13:42chamlis_ now invert a binary tree ;)
RoryOKane Maybe it would help to link from the parser to the MariaDB docs
RoryOKane that say the length of a shortword
RoryOKane next to the `repeat(4)`
I linked it from the test, yeah, next to the repeat for.
That's not a bad idea.
02:14:46chamlis_ big choice of 4 letter words for that test
I realized I didn't write the invert test as long as I was touching it.
Oh.
That's interesting.
02:15:03RoryOKane it's working anyway because expression matches term before shortword
Yeah, so this... Like, it composes okay because it comes after a term, but it feels like a foot gun.
Can I tell...
harshlet the maximum number yeah repeat okay so i can say min and max all right so i want to say repeat one comma three and then i'll get what i want i take a real boots and suspenders approach to search and search parser because i think of it as security sensitive code if it does the wrong thing there's the potential for sql injection
And it's a lot of custom code.
So all right.
Let's see what I just added.
Good.
And
02:16:15They're not really... There we go.
...30RoryOKane remember the comment next to `repeat(4)` β I think you were still considering it
Alright.
So how long was that bug hanging out that we just fixed?
A little over a year.
...45Yeah.
...58chamlis_ nice going!
All right.
Well, that feels nice to fix.
I really find that search engine satisfying to tinker with.
There's just something about the fact that it's such a clean computer science-y expression of the job, and it actually works.
And
The previous search engine was just kind of a series of string transformations with no real rigor behind it.
RoryOKane I guess your comment above `rule(:shortword)` already is enough guidance
That's not an insult.
It was just, it did the simplest thing and then it got a little more complicated over time.
And this one has been a real pleasure to extend where, yeah, Rory,
added the comment you suggested.
The one that's right above the cursor here, this one, I added that in response to your previous comment.
So anyways, doing this the right way has been so much more satisfying because there used to be feature requests for the search engine and I think a couple of them were just kind of closed as won't do because
They were so complicated.
And here, even when we get these bugs, handling input that the search engine can't do, I don't know, this feels like a really clean expression of what our limitation is.
And while it would be nice to have no limitation, I will settle for really clear help.
This feels almost like a very small Easter egg to me, getting to reuse the actual search term that the person had.
All right.
Good deal.
Ooh, wait, there was a story on concolic execution that I missed a month ago.
I will put this over on my browser so I can read that later.
02:19:21That's who is this?
...36can call it execution is a kind of testing strategy. That I studied a bunch when I was attending recurse center and The name is a combination of concrete and symbolic. And it's, I don't know, it's been kind of lingering at the edges of academia a while because it has some scaling issues but it promises to make property tests just about as comprehensive as mathematical proofs and hopefully run in a lot faster time than property testing. I don't know, to me it has a lot of promise. There's a lot of scaling work to do, but I would like to see it succeed. Not so much of actually working on it, but you know, it's neat.
02:21:01All right, so where am I on time? 2 hours 20. All right. So that's a good time to remember that or to do a reminder that this is Lobster's office hours. If you have any questions about the site or the code base, you can feel free to pipe up any time. And when people aren't asking questions, I work on the code base. And you just saw me running through some issues, hopefully without introducing any more. And then otherwise, I tend to work on stuff and... Hmm. Didn't I just finish one of these? On the last stream, I fixed one of these bugs.
...52Oh, I think I accidentally fixed this bug and I didn't realize I had recorded it.
02:22:05Oh, I've basically fixed this one. Yeah. So in revoked token, these errors get raised and we can just consume them. And this one actually fixes the primary thing that was reported. The other one is no IPs error. We will just say that doesn't resolve to an IP address. And then what was the other one?
...59This is probably, I didn't record the Ruby exception, but these are probably both going to be SSL errors.
02:23:20OK, so let's grab that about Mastodon. Where is it? I'm in the wrong repo.
...39There we go. So with that.
...55Need to have a short one.
02:25:22It's like I got a free bug fix right there. And that one, hey, I feel better about it. That one was only open for four months. These low priority bugs kind of take their time.
...49So where am I? Let's see. I'm at about two and a half hours on time and nobody's got questions. Oh, I was going to look at those. I was going to look at the comments, right? The links on comments. I was going to fix the styling. I'm a little reluctant to open the story merging thing because it is a lot more than a half an hour of work and I tend to go for about three hours. So maybe I'll just do this styling and call it a day.
02:26:41RoryOKane Iβm working on submitting a two-line formatting PR that was in my draft branch for fixing the comment preview bug you fixed
Rory, could you say some more about what your PR is for?
RoryOKane commit message: "fix indentation in _commentbox.html.erb"
It was a, it sounds like it's just something you ran into along the way.
02:27:02All right.
RoryOKane https://github.com/lobsters/lob…
So usually these should all have text decoration.
None.
That is the way they have classically done it.
Oh, new PR who this.
...30Ah, yeah. That's a pretty minor style thing. We don't have any kind of linting on the views. Sure.
02:28:18What's the one I just did?
RoryOKane there was no GitHub issue for it
I can't find the, oh, there wasn't an issue for it.
So I just need the SHA.
Yeah.
...55What is today? Oh, it's 3.10. For eager beavers, know that link is not live, but it will be in a couple hours when I render the archive. There we go.
02:29:30These this habit of just always, always linking to context and explaining what the heck I was thinking has been invaluable for me, especially now that I've been maintaining the code base for a few years. Cause I'm constantly running across old stuff and wondering like, wait, what was up with that? Now I look at it and it's like, oh, oh, okay. Now I know what's up with that. So this says delete has text decoration, none, and yet I conspicuously see and underline.
02:30:12So let's look at the computed style.
...19Where did you come from?
...28Any link has underline.
...40Byline says none. Why is this more specific?
...52Oh, this is my browser, I bet. I have my browser set to override and always underline links.
02:31:12Yep. Let's uncheck that. Hey, that's an easy bug fix, right? It's just my own browser. And the others aren't doing it because they don't have an href on them, because they're only usable by JavaScript, which there actually is a flow for editing. We could probably restore that. So let's go look at the comment box.
02:32:11If anybody wonders how GURP worked, I aliased it about a decade ago. If there is a typo you always make, just alias it to the right thing and move on. It is easier than fixing a typo that has hung out for ages.
02:33:14cache emptied and now the page load is going to take a minute there we go undefined method comment oh i needed the symbol or just the quotes because it really is a very stringy kind of thing okay so now well that mostly worked except did i typo the class what was the class comment underscore editor comment underscore editor the javascript must not have a prevent default
02:34:16Yep, because it's never had a. All right. So we'll just grab that from here. We'll say prevent default.
...34It is a little tempting with these actually. to move the prevent default to the end so that if it throws an exception, you go into the non-JavaScript path. But especially with the deleter, yeah, you enter an inconsistent state because what if it worked, but like the replace threw an exception, then you would go to a page that showed the comment was already deleted and that would be really confusing. So yeah, I'll leave it as is. Let's not get too clever about it.
02:35:16All right. So if I reload, I didn't undo my undo.
...43Good. But now edit. All right, progress. So now on the comment controller, instead of assuming
02:36:22I want to do it. You fail if I ask for a layout because I passed true. So are you looking for a layout named true? No. Let's just do it the long way.
02:37:10Daniel Katz- missing template comic box. Daniel Katz- Because it is a partial, but I want the layout.
...25Daniel Katz- missing local alright so pass it is. Daniel Katz- The whole thing with partials and whether they get a layout is not. has not aged super well. So now if I say comment v8, well, let's say 7 to 8, because there are two comments on the page. I want to make sure I know which one I'm updating. Well, that JavaScript fired, and then the page doesn't have, all right. Oh, boy. A lot of things that are assuming JavaScript. Let's go back to the comment. did edit people generally shouldn't be able to see that low I don't like that so yeah we click edit we get this and then this what if I preview okay that does work but then it's not in the li so the styling is a little messed up this is kind of a nice to have but i think i will take a partial win all right yeah cat you gotta lean on the mouse
02:39:19Oh, I should pre-fill the parents, shouldn't I?
...39Nice to have that context.
...46It puts it in a comment.
02:40:00I'm going to call that good enough. It is an improvement, and I can file an issue for the bug, and then maybe some kind generous soul will come along and fix it, because it's easier to fix a bug than add a feature. Or at least it's less intimidating. There's a known state you want to get into that tends to be a little more specified than feature work.
...31Yeah, let's bounce Rails server because I touched comment.
...41Give that a second.
02:41:01And I have to rebase onto Rory's.
...11OK. So then let's just file that issue.
...51Let's grab that commit.
02:42:54Did I? I should have checked update. Does update do the right thing? No, it just renders. OK, so there's two bugs.
02:45:02AnakimLuke will it not be funny when a hash spells out a real word and it gets confused on a message? :D
AnakimLuke In beeeeef I made it possible....
Anakim, yeah, that's actually a real... Oh, when I hash... Yeah, any six-letter word.
We actually also have that because we have these arbitrary short IDs.
It is possible for them to contain naughty words.
This is the Scunthorpe problem.
pushcx https://en.wikipedia.org/wiki/S…
Oh, and I've linked it before on stream.
So you are mentioning a sort of harmless version, but the Skunthorpe problem is when this collides badly with a censorship system that's not too bright.
02:47:13chamlis_ would be funny if twitch didn't let you send that link
So hopefully somebody sees that and goes, wow, what a great self-contained two little bugs.
I would love to fix those.
AnakimLuke my app has a 4-letter randomly generated url section that every day before bed time I pray for the gods of probability won't spell anything problematic
And then maybe next stream I will get to merge those fixes.
And I say next stream because I'm going to wind up.
...38Anakim, the easy fix there is to take all of the vowels out of your generation. There are still a few things that you could generate abbreviations that would be problematic, like KKK. But at least if you take out the vowels, you can't generate any of the really obvious curse words.
02:48:15All right.
You know what I just did?
I just pushed some code and I didn't run the build.
All right, what's the over-under on whether I just broke the build?
I think probably not.
Let's see what it says.
So if this passes, I'm going to wind up the stream.
So if you have any last questions about the CIDR codebase, throw them in chat real fast.
This takes about 30 more seconds.
And if I broke it, obviously I'll fix it and then I'll end the stream.
So you have only slightly more time to get questions in.
There we go.
All right.
chamlis_ special stream to commemorate blocking the UK?
So if that's all done and everything is synced, yeah, let's go ahead and deploy that fix and call it a day.
AnakimLuke EZ
Nice little stream.
Rory, thanks again for dropping in with your bug report.
I appreciate it.
And otherwise, folks, thanks for hanging out with me as I code on the site.
chamlis_ thanks
Yeah, let's close all of those.
Nice little day.
AnakimLuke byee
Alrighty, so the next scheduled stream is Thursday morning at 9 AM.
Oh, I better turn off the broad notice.
Bye, Anna Kim.
Nice to see you at the end here.
So that's 9 AM Chicago time on Thursday.
And the Twitch thing, iCal schedule mostly works, depending on how rigorous your
CalParser is.
Hope to see you on Thursday morning folks.
Take care.