Mark Bainter mbainter

mbainter commented on pull request neovim/neovim#2418
@mbainter

Let me know either way, I can't really say myself.

mbainter commented on pull request neovim/neovim#2418
@mbainter

Here in this function we have an ifdef that leverages using the bracketless if that affects the way the function behaves. Within the ifdef it sets …

mbainter commented on pull request neovim/neovim#2418
@mbainter

In this function, mustfree gets used a couple of different ways. I started working on trying to clean that up, but in my opinion the changes requir…

mbainter commented on pull request neovim/neovim#2418
@mbainter

I've worked out the changes. They were a little tricky in a couple places, so I'll call out some specifics someone should double check for me.

@mbainter
mbainter commented on pull request neovim/neovim#2418
@mbainter

I'd be happy to improve it further. I am just trying to avoid overreaching or introducing too much change at once. The concern makes sense to me, so …

mbainter commented on pull request neovim/neovim#2418
@mbainter

Hmmm - that isn't what's happening here I don't think -- or at least not what I intended. I'm returning the result of xstrdup -- so the only value …

@mbainter
@mbainter
mbainter commented on pull request neovim/neovim#2418
@mbainter

This was all me. If we return *p, then we could be returning a pointer to a value that is supposed to be a constant. Rather than try to refactor ev…

mbainter commented on pull request neovim/neovim#2418
@mbainter

Yes, for konst, from the style guide constant vars were supposed to have this yes? I suppose that is a good point on the name itself. The later pa…

mbainter commented on pull request neovim/neovim#2418
@mbainter

Crap - I knew the comment one. Forgot about that. I'll fix the braces...I knew I should've checked the style guide on that one, I had a nagging fee…

mbainter commented on pull request neovim/neovim#2418
@mbainter

Well, that's one hurdle. What about the actual work itself, is the refactor good, or does it look like some took a transcript of a couple arguing a…

mbainter commented on pull request neovim/neovim#2418
@mbainter

Running ":make test" from inside vim, all the tests pass. If I drop out, and run "make test" from the shell I get errors. Or rather -- I was gettin…

@mbainter
mbainter commented on pull request neovim/neovim#2418
@mbainter

Okay - this was a more ambitious effort than I thought I'd originally try to tackle - so if I went off the rails anywhere, please say so. I primari…

@mbainter
mbainter commented on pull request neovim/neovim#2418
@mbainter

Okay. I'll get working on merging my other branch with this one, and push it in later. I would definitely agree that a patch shouldn't introduce i…

@mbainter
@mbainter
mbainter commented on pull request neovim/neovim#2418
@mbainter

Like before, I was trying to control the number of changes I was making - and I wasn't entirely sure that change would be desired, so I didn't want…

mbainter commented on pull request neovim/neovim#2418
@mbainter

Yeah, I noted that somewhere earlier in this PR, that's not a good solution, but I was trying to limit my change to only be the u_char -> char. I'm…

mbainter commented on pull request neovim/neovim#2418
@mbainter

Sure, I'll address that.

mbainter commented on pull request neovim/neovim#2418
@mbainter

I figured we'd wait for that anyway -- thanks for the note on the quickbuild, I was just looking at that. I rebased this branch onto current master…

mbainter commented on pull request neovim/neovim#2418
@mbainter

Done.

@mbainter
@mbainter
mbainter commented on pull request neovim/neovim#2418
@mbainter

Is anything else needed from me before we can merge this?

@mbainter