Skip to content
New issue

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

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

Already on GitHub? # to your account

Make sure column names are always titlized #368

Closed
wants to merge 2 commits into from

Conversation

palm86
Copy link

@palm86 palm86 commented Aug 16, 2016

Here is a naive fix for #249 . I'd be happy to dig in some more. It works for me.

@jieter
Copy link
Owner

jieter commented Aug 16, 2016

Thanks for looking into this, however, before this can be merged, tests have to pass. Can you have a careful look at them?

@palm86
Copy link
Author

palm86 commented Aug 16, 2016

I've had a quick look. I have to admit that the intent of some of the tests is not clear to me. And there are some that I don't agree with on first glance. I will have a more careful look. In the meantime, can you take a look at what I've done? As I said it is probably a very naive fix (not meant for merging just yet) and maybe you can also provide some pointers.

@@ -434,7 +434,7 @@ def verbose_name(self):

# This is our reasonable fallback, should the next section not result
# in anything useful.
name = title(self.name.replace('_', ' '))
name = self.name.replace('_', ' ')
Copy link
Owner

@jieter jieter Aug 16, 2016

Choose a reason for hiding this comment

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

If this change is applied, the second part of the first sentence of the docstring of this method is not true anymore. Please look carefully at the docstrings too when modifying these kinds of things.

@jieter
Copy link
Owner

jieter commented Aug 16, 2016

This might reflect assumptions made before things were clarified like in #249, and should be changed accordingly.

However, I think we should not always titlize. If I provide an explicit verbose_name for a column, I want the value in the header to be just that, not changed by some magic inside django-tables2.

@jieter
Copy link
Owner

jieter commented Sep 6, 2016

@palm86 did you see my comments?

@jieter
Copy link
Owner

jieter commented Sep 9, 2016

#382 is a better approach, closing this one in favour of that PR.

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

Successfully merging this pull request may close these issues.

2 participants