Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Shell: support bell and buffer incomplete UTF-8 sequences #7979

Merged
merged 4 commits into from Feb 10, 2018

Conversation

bfredl
Copy link
Member

@bfredl bfredl commented Feb 7, 2018

actually fixes #4338. I will take a peek at gvim source code if there is other low hanging fruit. But more complex stuff (like colors) should be punted until :terminal is integrated.

I also merged bang_filter_spec into output_spec as in fact only tests bang. Filter is tested elsewhere.

@bfredl
Copy link
Member Author

bfredl commented Feb 7, 2018

Actually our execute('!ls') behavior is consistent with gvim which is inconsistent with terminal vim.
gvim does one more thing: it makes sure to buffer an incomplete multibyte character, we should do that as well.

@marvim marvim added the WIP label Feb 7, 2018
@bfredl bfredl force-pushed the shellbell branch 2 times, most recently from b811c45 to 3d26213 Compare February 8, 2018 20:38
@janlazo
Copy link
Contributor

janlazo commented Feb 9, 2018

@bfredl Can you enable the tests from bang_filter_spec.lua on Windows? I did it in janlazo@6e077b2#diff-8cc34a316e32ee95eb86a21b308e7082

@bfredl
Copy link
Member Author

bfredl commented Feb 9, 2018

Thanks, I cherry picked it.

@bfredl bfredl changed the title [WIP] Shell: support bell [RFC] Shell: support bell and buffer incomplete UTF-8 sequences Feb 9, 2018
@bfredl
Copy link
Member Author

bfredl commented Feb 9, 2018

Failures looks unrelated (nodejs...), marking RFC.

@bfredl bfredl merged commit 34b99bc into neovim:master Feb 10, 2018
@jszakmeister jszakmeister removed the RFC label Feb 10, 2018
@@ -131,4 +139,91 @@ describe("shell command :!", function()
]])
feed([[<CR>]])
end)

describe('', function()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe("shell command :!", function()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that is the parent of this block. If I added it it would be echoed twice in the test description. This is just a bunch of tests that share common initialization, is there another canonical way for that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake, it looked like the same indent level as line 80. I would normally put it in its own scope but doesn't matter I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zero width control characters insert spaces with shell ex commands
5 participants