Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

Breadcrumb helper function #346

Merged
merged 8 commits into from
May 27, 2015
Merged

Conversation

Larzans
Copy link
Contributor

@Larzans Larzans commented May 25, 2015

Adapted the breadcrumb solution by thewebtaylor (http://thewebtaylor.com/articles/wordpress-creating-breadcrumbs-without-a-plugin) for FoundationPress

@olefredrik
Copy link
Owner

Hi. Thanks for your pull request. Unfortunately the Travis CI build failed. Could you check the details, fix the issues and add push a new commit? https://travis-ci.org/olefredrik/FoundationPress/jobs/63924272

Thanks.

@Larzans
Copy link
Contributor Author

Larzans commented May 26, 2015

Sure, i hope this fixes that :)

@olefredrik
Copy link
Owner

Hm. I'm unable to restart the travis build because the pull request can no longer be automatically merged. There seems to be merge conflicts that needs to be resolved first.

@Larzans
Copy link
Contributor Author

Larzans commented May 26, 2015

Hm, ok, makes sense, i used PHPCBF to remove the travis errors, it also changed the rest of the content in the file to make it more standard conform.
Let me revert and just add the new 'clean' function, just a minute.

@Larzans
Copy link
Contributor Author

Larzans commented May 26, 2015

Ok, it looks like my ide changed the file even before the PHCBF, sorry about all the trouble, i will just overwrite it with the master navigation.php and then add the new function with another ide.

@Larzans
Copy link
Contributor Author

Larzans commented May 26, 2015

Eherm. i am using the wrong rules for phpcbs :|
Gonna fix that and commit again.

@Larzans
Copy link
Contributor Author

Larzans commented May 26, 2015

I am sorry, but i have problems with my codesniffer setup, could you help me please?
I installed the wpcs repository and am using it together with the codesniffer.ruleset.xml from foundationpress, but it shows me different errors from the one in the Travis builds.

Is there anything else i have to take into account?

The command line looks like this:
phpcs.bat -p -s -v -n --standard="FoundationPress\codesniffer.ruleset.xml" "FoundationPress\library\navigation.php"

@olefredrik
Copy link
Owner

Hello. To be honest I'm quite new with WordPress Coding Standards. Personally, I believe that many of the rules are too rigid. It's not like we're hacking the WP Core, after all. I've added the most useless rules to the exclusion list. I haven't tried setting up codesniffer for automatic correction of errors myself, actually. I fix all my errors manually. For me it makes no sense that codesniffer should show you different errors from the one in the Travis builds, if you copied my codesniffer ruleset. That's really strange. One option would be to fix the errors manually. It's insanely boring, I know. But I suppose it will be worth it when the build eventually passes.

@Larzans
Copy link
Contributor Author

Larzans commented May 26, 2015

Haha, glad that you said that, i started to feel quite stupid with all the rules i supposedly broke :)
I will test a bit more tomorrow and if nothing werks i will bite the bullet and do it manually too, i just couldn't bring myself to do it as i thought it might be useful in the future to get the automatic wp-rule-purge up and running, but i do think as well that some rules are a bit too strict / exaggerated.
Cross my fingers that it does not come to that though, but i am almost out of ideas...

@Aetles
Copy link
Contributor

Aetles commented May 26, 2015

Just a side note: As being one of those who enjoyed the effort of having FoundationPress following the WP code standard I'd say it's more about consistency than anything else (and the belief that consistent code is easier to read, easier to automatically review and could prevent errors). Could have been another code standard for all I care. I'm not particularly keen on the WP code standard from a personal point of view (took quite a while to get used to all those spaces) but since this is part of the WP world it does seem like a natural choice.

@Larzans
Copy link
Contributor Author

Larzans commented May 27, 2015

Finally! Kinda satisfying i have to admit :)

olefredrik added a commit that referenced this pull request May 27, 2015
@olefredrik olefredrik merged commit d8137f3 into olefredrik:master May 27, 2015
@olefredrik
Copy link
Owner

Brilliant! Thanks a lot! :)

@Larzans Larzans deleted the add_breadcrumb branch May 27, 2015 16:12
@Larzans
Copy link
Contributor Author

Larzans commented May 27, 2015

Great, thanks for this wonderful project, saved me tons of time :)

@aibrean
Copy link

aibrean commented Jun 24, 2015

Doesn't work with custom post types though.

@Larzans
Copy link
Contributor Author

Larzans commented Jun 25, 2015

Hmm, didn't test it with custom post types, i will check that out.

@AdamChlan
Copy link
Contributor

I feel like I remember seeing an email coming across saying that CPTs were now supported with breadcrumbs, but I'm unable to find it in the issue tracker. Was this just my imagination?

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

Successfully merging this pull request may close these issues.

None yet

5 participants