python-caldav / caldav

Apache License 2.0
319 stars 94 forks source link

Added get_name convenience method for Calendar #212

Closed mc-borscht closed 2 years ago

mc-borscht commented 2 years ago

Requested get_name convenience method created for Calendar object.

Returns name object attribute.

Docstring indicates that a None object is returned if a name is not set - maybe this is beyond the scope of this method's knowledge though?

tobixen commented 2 years ago

One would expect Calendar.get_name() to yield the "display name" if such a thing is set at the server side. The self.name attribute is some artifact which is only set if given by the client. Ref #128 self.name is going to be removed in a future release.

I don't mind having such a convenience method, but it should do the right thing - and that is to do a call on the get_property method.

(This comment supersedes my previous comment, so I will delete it)

mc-borscht commented 2 years ago

Agreed, makes sense.

  1. I've re-named the method to get_display_name which follows the desired behavior - this was a speculative change on my part.
  2. It now follows the desired behavior, as described above.
  3. Replaced all calls to get_property(dav.DisplayName()) with calls to the get_display_name method.

Let me know if anything else required or if you want me to change the method name back to get_name

tobixen commented 2 years ago

Don't worry about style test not passing. I believe it can be auto-resolved by running tox -e style. I will eventually do it at some point, but of course for annotation purposes it's better that you do it.

mc-borscht commented 2 years ago

Minor formatting changes were carried out by tox -e style command as suggested - it passes when I re-run the command.

tobixen commented 2 years ago

There is still a bug on line 401

mc-borscht commented 2 years ago

Fixed the following issues and passed style test

Hope this closes things - sorry about that!