Skip to content

Implement debug.List - #9163

Open
mohsalsaleem wants to merge 4 commits into
gohugoio:masterfrom
mohsalsaleem:debug-list-9148
Open

Implement debug.List#9163
mohsalsaleem wants to merge 4 commits into
gohugoio:masterfrom
mohsalsaleem:debug-list-9148

Conversation

@mohsalsaleem

Copy link
Copy Markdown

Refers #9148
This PR implements debug.List method that uses reflect package to to list exported fields and methods names on structs/pointers and all keys in a map.

@CLAassistant

CLAassistant commented Nov 12, 2021

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@bep

bep commented Nov 12, 2021

Copy link
Copy Markdown
Member

Thanks, can you push the "sign CLA button" above?

@mohsalsaleem

Copy link
Copy Markdown
Author

I’ve completed the CLA Signing, but for some reason the bot thinks I have not.

@bep

bep commented Nov 12, 2021

Copy link
Copy Markdown
Member

I’ve completed the CLA Signing, but for some reason the bot thinks I have not.

You have probably a email mismatch between the mail address you used for the commit (as in: your Git user) and the email address registered on GitHub. You can fix this by adding email aliases in your GitHub profile.

@mohsalsaleem

Copy link
Copy Markdown
Author

Thanks for pointing it out, looks like CLA check passes now!

@mohsalsaleem

Copy link
Copy Markdown
Author

In case I need to add tests, please let me know. I looked around and could not find anything specific to add tests to apart from the init.go file, which I'm unclear on how to structure the example.

@bep

bep commented Nov 12, 2021

Copy link
Copy Markdown
Member

In case I need to add tests, please let me know.

We need some basic tests. Just create a debug_test.go file and put a basic TestList test with a simple test structs and a map. The init test you can ignore.

@mohsalsaleem
mohsalsaleem marked this pull request as ready for review November 13, 2021 15:35
@mohsalsaleem

Copy link
Copy Markdown
Author

@bep I just added some basic tests, please have a look! Thanks.

@mohsalsaleem

Copy link
Copy Markdown
Author

I just noticed that the functions also returns fields that are not exported, let me fix that.

@mohsalsaleem

Copy link
Copy Markdown
Author

Fixed the issue, please review.
Thanks.

@bep

bep commented Nov 21, 2021

Copy link
Copy Markdown
Member

Thanks for this. I need to think a little before pulling this into the main branch. I have tested this a little, and while it does work great on technical level, I fear it may expose too much (deprecated and internal methods (not meant to be used in the templates, e.g. MarshalJSON) which I think may be more confusing than useful. I need to think about a way to filter out those.

@bep bep added the Keep label Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants