Skip to content

Add Buffer.__repr__ and Window.__repr__#328

Merged
bfredl merged 1 commit intoneovim:masterfrom
blueyed:repr
May 22, 2018
Merged

Add Buffer.__repr__ and Window.__repr__#328
bfredl merged 1 commit intoneovim:masterfrom
blueyed:repr

Conversation

@blueyed
Copy link
Copy Markdown
Contributor

@blueyed blueyed commented Apr 26, 2018

No description provided.

@justinmk
Copy link
Copy Markdown
Member

somewhat related, "lgtm" service also noticed some other things: https://lgtm.com/projects/g/neovim/python-client/alerts/

@bfredl
Copy link
Copy Markdown
Member

bfredl commented Apr 26, 2018

As an alternative, could add a generic __repr__ in the Remote base class and cover all three objects.

@blueyed blueyed changed the title Add Buffer.__repr__ Add Buffer.__repr__ and Window.__repr__ May 6, 2018
@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented May 6, 2018

@bfredl

As an alternative, could add a generic repr in the Remote base class and cover all three objects.

They have different properties though.

I've implemented it now for all three.

@bfredl
Copy link
Copy Markdown
Member

bfredl commented May 6, 2018

@blueyed They all have the same essential property: the object handle. Also I'm not sure we want __repr__ to use sync requests: one should be able to print object representations in a debug statement, even at a place where sync requests are not safe.

@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented May 6, 2018

They all have the same essential property: the object handle

Added this now to all of them.

use sync requests

Thought about this as well.
Removed the filetype for buffer's filetype.

@bfredl
Copy link
Copy Markdown
Member

bfredl commented May 6, 2018

Note that most other attributes than the handle requires sync requests. What I had in mind was something like

def __repr__(self):
       """Get text representation of the object."""
       return '<pynvim.{}(handle={})>'.format(self.__class__.__name__,
                                             self.handle)

which can be in the Remote class.

@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented May 6, 2018

Note that most other attributes than the handle requires sync requests

Than I am more in favor of including them, but protecting them with a try/except then instead.

Most of the current state's information is useful after all.

@blueyed blueyed closed this May 6, 2018
@blueyed blueyed reopened this May 6, 2018
@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented May 6, 2018

Added a commit to remove remote calls altogether.

Comment thread neovim/api/buffer.py Outdated
def __repr__(self):
"""Get text representation of the buffer."""
return 'Buffer(number=%r, handle=%r)' % (
self.number,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

number and handle are synonymous. After this, we can just as well have a shared implementation in Remote.

@justinmk
Copy link
Copy Markdown
Member

justinmk commented May 9, 2018

LGTM, but tests are failing.

file /home/travis/build/neovim/python-client/test/test_buffer.py, line 6
  def test_repr(request, vim):
file /home/travis/build/neovim/python-client/test/conftest.py, line 70
  @pytest.fixture
  def no_remote_calls(vim, mocker):
E       fixture 'mocker' not found
>       available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, cleanup_func, doctest_namespace, monkeypatch, no_remote_calls, pytestconfig, record_property, record_xml_attribute, record_xml_property, recwarn, tmpdir, tmpdir_factory, vim, worker_id
>       use 'pytest --fixtures [testpath]' for help on them.

@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented May 9, 2018

Do you think the no_remote_calls fixture is worth the pytest-mock dependency?
If so I'll fix the build/installation.
/cc @bfredl
Also note that I haven't included the suggested pynvim. prefix, which is not typical with __repr__ methods AFAIK.

@bfredl
Copy link
Copy Markdown
Member

bfredl commented May 9, 2018

Lets skip the mock test for now, it seems overcomplicated for only this functionality.

The output is probably fine as it is. The reason for the < and > would say that it is non-reconstructible (unlike repr(json_like_object) ) but we can skip that, it should be understood anyway.

@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented May 21, 2018

Amended, and added the <…>.

Comment thread test/test_buffer.py


def test_repr(vim):
assert repr(vim.current.buffer) == "<Buffer(handle=2)>"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hmm, looks a bit fragile, but test run order seems deterministic. So guess we can keep this for now and change it to a regex later if it turns out to be too much churn.

@bfredl bfredl merged commit 3777747 into neovim:master May 22, 2018
@blueyed blueyed deleted the repr branch May 22, 2018 17:32
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.

3 participants