Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[VarDumper] Add dd() helper == dump() + exit()#26970

New issue

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

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromnicolas-grekas:dd
Apr 19, 2018

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

By popular demand, I feel like we should reconsider our refusal for add() global helper.
For past references, see#26965,#26906,#13657,#17267,#19096.

wouterj, javiereguiluz, linaori, mathieutu, maxbeckers, IfnotFr, alexandre-tobia, dkarlovi, decima, cbastienbaron, and 131 more reacted with thumbs up emojistloyd, jvasseur, kunicmarko20, curry684, jderusse, donis, tdutrion, Shine-neko, dknx01, julienfalque, and 47 more reacted with thumbs down emojiklapuch, carusogabriel, dinhquochan, skmetaly, and aureliengiry reacted with laugh emojidamienalexandre, javiereguiluz, mathieutu, IfnotFr, decima, cbastienbaron, vinkla, nunomaduro, seyfer, welcoMattic, and 12 more reacted with hooray emojideflock, goetas, Nek-, hhamon, rybakit, teresko, pomaxa, paq85, lookyman, JANorman, and 10 more reacted with confused emoji
Copy link
Member

@javiereguiluzjaviereguiluz left a comment

Choose a reason for hiding this comment

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

We gave some reasons to not add this function in the past ... but when you receive so many requests from the community to add it, it's time to rethink your decisions. Nicolas, thanks for reconsidering this and for adding the function.

@onEXHovia
Copy link
Contributor

I think there will be a similar number of pros and cons.
Should we addTwigFunction by analogy withdump 🤔

@pamil
Copy link
Contributor

Seeingdd() anywhere in the code for the first time would cause nothing else but confusion. If one wants to save keystrokes, why not create a macro in IDE (or use a debugger)?

kunicmarko20, jvasseur, Shine-neko, stloyd, Taluu, kevin-verschaeve, goetas, thomasruiz, Eliaenor, rybakit, and 22 more reacted with thumbs up emojihopeseekr, kostaspt, jordonbaade, eugenefvdm, davidstillson, reinierkors, deleugpn, OwenMelbz, quentin-st, shalvah, and 4 more reacted with thumbs down emoji

@linaori
Copy link
Contributor

  • It doesn't add complexity
  • It's optional
  • It's faster than hooking up the debugger
  • Not everyone has experience with a debugger
  • It doesn't replace a debugger and you're not forced to use this

I see no downsides and maintenance isn't an issue 👍

mathieutu, javiereguiluz, IfnotFr, cbastienbaron, bhulsman, frankdejonge, lyrixx, Pierstoval, bourvill, Tiriel, and 54 more reacted with thumbs up emojiteresko, diwms, malios, Bilge, nicwortel, fesor, fmata, jorge07, veloxy, goetas, and 5 more reacted with thumbs down emojiteresko, pomaxa, and Bilge reacted with confused emoji

@kunicmarko20
Copy link

kunicmarko20 commentedApr 18, 2018
edited
Loading

@iltar But you havedump(), as@pamil said you can create IDE macro. If this gets accepted won't it start an avalanche of people wanting shortcuts for something?

rybakit, OskarStark, fmata, foyb, inverse, zanbaldwin, alsoknownasdrew, Wirone, and DHoogland reacted with thumbs up emojigarf, garygreen, wwwroth, hopeseekr, OwenMelbz, connor11528, IckleChris, and fulopattila122 reacted with thumbs down emoji

@javiereguiluzjaviereguiluz changed the title[VarDumper] Add dd() helper == dump() + die()[VarDumper] Add dd() helper == dump() + exit()Apr 18, 2018
@lsmith77lsmith77 self-requested a reviewApril 18, 2018 09:43
Copy link
Contributor

@lsmith77lsmith77 left a comment

Choose a reason for hiding this comment

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

I think the potential for harm is very small and the feature has been requested a lot.

Copy link
Member

@lyrixxlyrixx left a comment

Choose a reason for hiding this comment

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

👍

@Pierstoval
Copy link
Contributor

Creating a macro in the IDE is part of the doc, not the framework, and not asked as much asdd(). I'm totally approving this feature as it's almost like "everyone's asking for it", where debugger usage, even though better, can be cumbersome for lots of devs.

garygreen, hopeseekr, and fulopattila122 reacted with thumbs up emojiAllenJB, OwenMelbz, adelowo, and DHoogland reacted with thumbs down emoji

stof
stof previously requested changesApr 18, 2018
Copy link
Member

@stofstof left a comment

Choose a reason for hiding this comment

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

SourceContextProvider must be updated to whitelistdd as well when looking for the source of the dump. Otherwise, the dump will be reported as coming from insidedd which is useless.

@linaori
Copy link
Contributor

@kunicmarko20 Not everyone is always developing with an IDE, don't forget the notepad(++), Vim, atom etc developers. Also important to note down, is that the var-dumper can be used as stand-alone. Meaning if devs like the symfonydump(), they can use this package. This reaches far beyond experienced developers with debuggers, as you'll probably be reaching hobby devs, wordpress/joomla/drupal devs etc.

When you're comparing the full feature set of the var-dumper component vs other dumpers, then a dump and die functionality is quite common and desired. It takes a few lines to add the function, but adds a big feeling of completion for the developers that would otherwise have used add()-similar function from another framework.

I think it's quite nice to have a "full" solution coming from Symfony, making it even better 👍

fweber-de, javiereguiluz, nicolas-grekas, Capenus, garygreen, elementaire, jkniest, OwenMelbz, Great-Antique, fulopattila122, and kevin92dev reacted with thumbs up emojiAllenJB reacted with thumbs down emoji

@pamil
Copy link
Contributor

After reading the reasoning behind introducing this function I think it's a good idea in fact and with almost no potential for harm. 👍

javiereguiluz, benjamincrozat, Capenus, hopeseekr, jordonbaade, ryanwinchester, reinierkors, Lutacon, OwenMelbz, jarektkaczyk, and sstok reacted with thumbs up emoji

@deflock
Copy link

The feature itself is must have, but not sure about name, confuses a bit.

@iltar Can you explain "It's optional"? What if I wantdump() but notdd() in global scope?

@Meroje
Copy link

@nicolas-grekas @DojoGeekRA chrome 65 removed the need for an error code to render the preview that was introduced with 62https://bugs.chromium.org/p/chromium/issues/detail?id=785050

garygreen, roberto-aguilar, nicolas-grekas, and nunomaduro reacted with thumbs up emojiroberto-aguilar, nunomaduro, Wirone, and sstok reacted with hooray emoji

@haydenk
Copy link

Why not add an optional argument todump() to exit when it's done instead of creating a whole new function?

@lyrixx
Copy link
Member

Why not add an optional argument to dump() to exit when it's done instead of creating a whole new function?

because dump is variadic

fulopattila122, Wirone, sstok, hhamon, and rvanlaak reacted with thumbs up emoji

@haydenk
Copy link

@lyrixx Ah, good point.

@joubertredrat
Copy link

joubertredrat commentedApr 18, 2018
edited
Loading

Oh, a enem..., erm, a friend@taylorotwell here? Hey, let's drink coffee?

About function, I think that is best to be more verbosity, howdump_die,dump_exit ordump_output as example, what you think guys?

Why not add an optional argument to dump() to exit when it's done instead of creating a whole new function?

Becausedump today support multiple arguments.

kunicmarko20, lyrixx, chapeupreto, greg0ire, nesl247, nunomaduro, vinkla, carusogabriel, fulopattila122, and Wirone reacted with thumbs down emoji

@kunicmarko20
Copy link

If people think we need a shortcut then it should be small,dd is the best choice for this. If it is longer then I can writedump();die(); myself.

exfriend, allanfreitas, sroze, OwenMelbz, edbizarro, Great-Antique, nztim, samlev, Wirone, and rvanlaak reacted with thumbs up emoji

@Pierstoval
Copy link
Contributor

What a mess of a conversation just for a DX initiative that has been demanded for a long time, and still lots of arguments against.

I think this is part of the small changes that can really conciliate lots of dev communities and improve "classic" debug experience as well for people not using complex xdebug setups or full-stack IDEs. I mean, when one debugs using VIM, a simpledd() is really straightforward.

theofidry, nesl247, tomschlick, AkenRoberts, OwenMelbz, edbizarro, nztim, fulopattila122, Wirone, sstok, and rvanlaak reacted with thumbs up emoji

@akalongman
Copy link

+1 fordump_die()

carusogabriel, nunomaduro, samlev, tgalopin, theofidry, kunicmarko20, fulopattila122, Pierstoval, and nicoeg reacted with thumbs down emoji

@Majkl578
Copy link
Contributor

Maybe renameSymfony\Component\VarDumper\VarDumper::dump() toD::d() instead?

ping-localhost and Pierstoval reacted with thumbs down emojitheofidry, michaeldyrynda, and zalexki reacted with confused emoji

Copy link
Contributor

@carusogabrielcarusogabriel left a comment

Choose a reason for hiding this comment

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

🍿

@antoniocambados
Copy link

I don't miss add() shortcut but I can barely think of any serious objection. The proposed name is convenient and matches other frameworks, so I'd stick with it.

@fabpot
Copy link
Member

Thank you@nicolas-grekas.

mdeltour and gragio reacted with thumbs up emoji

@fabpotfabpot merged commita55916a intosymfony:masterApr 19, 2018
fabpot added a commit that referenced this pull requestApr 19, 2018
…s-grekas)This PR was merged into the 4.1-dev branch.Discussion----------[VarDumper] Add dd() helper == dump() + exit()| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -By popular demand, I feel like we should reconsider our refusal for a `dd()` global helper.For past references, see#26965,#26906,#13657,#17267,#19096.Commits-------a55916a [VarDumper] Add dd() helper == dump() + die()
@Wirone
Copy link
Contributor

@Pierstoval

I mean, when one debugs using VIM, a simple dd() is really straightforward.

Unless they're in command mode 😂

sstok, nicolas-grekas, and cjean-wishibam reacted with laugh emoji

@sstok
Copy link
Contributor

sstok commentedApr 19, 2018
edited
Loading

Lets be clear, a function/method/class should always have a name that describes it's purpose, it helps the developers and prevents mistakes.

dd, a or Foobar are not descriptive (unless you have a Bar named foo:trollface: - insert Jonh Taffer meme here ) but it's important to understand why we use these undescriptive names, because it doesn't matter. Foobar is only used as an example for testing,dd() is something you only use during development!! It prints the result and stops execution, please don't tell me your production does this kind of logic 😐 (you properly use a proper solution for a specific problem and let the framework handle exceptions and the response handling).

Clean Code is about production code and tests, not about simple hacks to find out why something isn't working. Not everyone has the power of a debugger, sometimes the debugger is not able to reach the code (actually happened to me a few time) or doesn't show all details (or the details are to verbose), by using the VarDumper you can easily solve this problem, and having a simple method to dump and prevent further execution is perfectly fine for development and troubleshooting.

fsevestre, Wirone, kunicmarko20, Pierstoval, javiereguiluz, nicolas-grekas, pimolo, zalexki, rvanlaak, and benjaminjonard reacted with thumbs up emoji

@gragio
Copy link

Good job! Thanks@nicolas-grekas

alessiodionisi reacted with hooray emoji

@jpauli
Copy link

👍

@nicolas-grekas
Copy link
MemberAuthor

I'm locking the thread because the issue has been resolved.
Thank you all for participating!

@symfonysymfony locked asresolvedand limited conversation to collaboratorsApr 19, 2018
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@lyrixxlyrixxlyrixx approved these changes

@javiereguiluzjaviereguiluzjaviereguiluz approved these changes

@stofstofstof left review comments

+15 more reviewers

@COilCOilCOil left review comments

@garygreengarygreengarygreen left review comments

@tomasfejfartomasfejfartomasfejfar left review comments

@nicwortelnicwortelnicwortel requested changes

@ogizanagiogizanagiogizanagi approved these changes

@lsmith77lsmith77lsmith77 approved these changes

@srozesrozesroze approved these changes

@joubertredratjoubertredratjoubertredrat approved these changes

@michaelcullummichaelcullummichaelcullum approved these changes

@Oliboy50Oliboy50Oliboy50 approved these changes

@antoniocambadosantoniocambadosantoniocambados approved these changes

@nunomaduronunomaduronunomaduro approved these changes

@egircysegircysegircys approved these changes

@unckleguncklegunckleg approved these changes

@carusogabrielcarusogabrielcarusogabriel approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.1

Development

Successfully merging this pull request may close these issues.

43 participants

@nicolas-grekas@onEXHovia@pamil@linaori@kunicmarko20@Pierstoval@deflock@hhamon@pimolo@houssemz@EmilCataranciuc@sailingdeveloper@teresko@stof@roberto-aguilar@javiereguiluz@tomasfejfar@taylorotwell@Meroje@haydenk@lyrixx@joubertredrat@akalongman@Majkl578@antoniocambados@fabpot@Wirone@sstok@gragio@jpauli@COil@michaelcullum@lsmith77@sroze@nicwortel@garygreen@ogizanagi@Oliboy50@nunomaduro@egircys@unckleg@carusogabriel@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp