Skip to content

Tracy\Bar's output can be disabled (closes #82) - #83

Closed
hranicka wants to merge 2 commits into
nette:masterfrom
hranicka:master
Closed

Tracy\Bar's output can be disabled (closes #82)#83
hranicka wants to merge 2 commits into
nette:masterfrom
hranicka:master

Conversation

@hranicka

Copy link
Copy Markdown
Contributor

Closes #82

@JanTvrdik

Copy link
Copy Markdown
Contributor

+1, although public $enabled = TRUE is probably enough

Btw: the commit message should probably be: "Bar: output can be disabled [Closes #82]"

Comment thread src/Tracy/Bar.php Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both functions take no arguments

@hranicka

Copy link
Copy Markdown
Contributor Author

Thanks @JanTvrdik , I've changed the commit message and fixed PhpDoc.


@JanTvrdik
I prefer public methods (::disable/::enable) instead of public property because in the future Bar can be interface and PHP interface cannot define public properties.

But I can change it to only one method eg. public function enable($enabled = TRUE).
What do you think about it?

@dg

dg commented Nov 25, 2014

Copy link
Copy Markdown
Member

What about simple method hide?

@hranicka

Copy link
Copy Markdown
Contributor Author

I've replaced the two methods with one hide($hidden = TRUE).

But "disable" sounds me better than "hide"... maybe...

And I also thinking about option for enabling the Bar again when is disabled. Maybe it's not necessary to have this option.

@dg

dg commented Nov 25, 2014

Copy link
Copy Markdown
Member

Maybe disable sound better… The hide was only idea to discuss.

@EdaCZ

EdaCZ commented Dec 21, 2014

Copy link
Copy Markdown
Contributor

Whats the status of this, please?

@bazo

bazo commented Jan 3, 2015

Copy link
Copy Markdown

please merge this. disable/enable or hide/show are both good names. but it's a pain in the ass to develop a rest api with the bar output in the response

@dg

dg commented Jan 3, 2015

Copy link
Copy Markdown
Member

@bazo Send the correct Content-Type header.

@bazo

bazo commented Jan 3, 2015

Copy link
Copy Markdown

ok, that helped. but still, this can be a useful feature

@dg
dg force-pushed the master branch 11 times, most recently from 9151150 to 00083f3 Compare February 10, 2015 03:47
@dg dg added this to the v2.4 milestone Feb 17, 2015
@dg
dg force-pushed the master branch 4 times, most recently from 0203000 to 0203000 Compare February 24, 2015 23:20
@encero

encero commented May 22, 2015

Copy link
Copy Markdown

+1, i am already using forked version with this merged.

@hranicka

Copy link
Copy Markdown
Contributor Author

Wait for v2.4 :)

@dg
dg force-pushed the master branch 3 times, most recently from 13f9a7b to 0c0f9d4 Compare July 12, 2015 10:17
@dg
dg force-pushed the master branch 2 times, most recently from 3495a54 to 8517fdc Compare October 5, 2015 11:45
@dg
dg force-pushed the master branch 7 times, most recently from 5bee3a1 to 42ed404 Compare November 5, 2015 00:41
@dg dg closed this in 622221f Jan 11, 2016
dg added a commit that referenced this pull request Jan 11, 2016
dg added a commit that referenced this pull request Jan 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracy\Bar cannot be disabled

6 participants