Skip to content

[Console] Add hyperlinks support #29168

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 10, 2018
Merged

Conversation

ostrolucky
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #21207
License MIT
Doc PR

For details about this see https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda

Here's one idea/use case which could utilize this feature (not implemented in this PR as it requires more work)
stack trace with anchors

I checked this in cmd.exe as well and no sideffects there

@enumag
Copy link
Contributor

enumag commented Nov 10, 2018

Oh you actually went and figured it out. Great work @ostrolucky! 👍

@nicolas-grekas
Copy link
Member

Wow! We need this in CliDumper also!

@nicolas-grekas nicolas-grekas added this to the next milestone Nov 11, 2018
@Koc
Copy link
Contributor

Koc commented Nov 12, 2018

Does it support framework.ide configuration option?

@ostrolucky
Copy link
Contributor Author

This PR doesn't use this functionality, it only enables its usage. Screenshot was just potential scenario, it would require work outside the scope.

@nicolas-grekas
Copy link
Member

Do we have any case where wrapping could need using the "id" attribute described in the linked doc?

@ostrolucky
Copy link
Contributor Author

I don't

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM with one minor comment.
Can you please add a line in the changelog file of the component + create a doc issue/PR?

@fabpot
Copy link
Member

fabpot commented Dec 10, 2018

@ostrolucky Can you submit a PR on the docs to document this new feature?

@fabpot fabpot force-pushed the console-hyperlinks branch from f1d658d to db750ed Compare December 10, 2018 03:47
@fabpot
Copy link
Member

fabpot commented Dec 10, 2018

Thank you @ostrolucky.

@fabpot fabpot merged commit db750ed into symfony:master Dec 10, 2018
fabpot added a commit that referenced this pull request Dec 10, 2018
This PR was squashed before being merged into the 4.3-dev branch (closes #29168).

Discussion
----------

[Console] Add hyperlinks support

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21207
| License       | MIT
| Doc PR        |

For details about this see https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda

Here's one idea/use case which could utilize this feature (not implemented in this PR as it requires more work)
![stack trace with anchors](https://user-images.githubusercontent.com/496233/48305600-00d4c300-e52e-11e8-94e6-33713ff09d50.png)

I checked this in cmd.exe as well and no sideffects there

Commits
-------

db750ed [Console] Add hyperlinks support
nicolas-grekas added a commit that referenced this pull request Dec 13, 2018
…s-grekas)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[VarDumper] add support for links in CliDumper

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Same as #29168 for VarDumper:

![capture d ecran de 2018-11-15 20-10-08](https://user-images.githubusercontent.com/243674/48576196-24c04c00-e914-11e8-8a61-c1304c876243.png)

Thanks @ostrolucky for this nice discovery!

Commits
-------

e7cd44f [VarDumper] add support for links in CliDumper
nicolas-grekas added a commit that referenced this pull request Dec 20, 2018
This PR was merged into the 4.3-dev branch.

Discussion
----------

[VarDumper] Use hyperlinks in CliDescriptor

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | no
| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | Part of #29585   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

Leverages #29168 to add the hyperlink directly on the source text instead of printing below:

#### Before (or with `symfony/console < 4.3`)

![capture d ecran 2018-12-14 a 16 37 18](https://user-images.githubusercontent.com/2211145/50012891-1e2efe00-ffc0-11e8-9e07-b5358cb057bd.png)

#### After

![capture d ecran 2018-12-14 a 16 12 13](https://user-images.githubusercontent.com/2211145/50012921-25560c00-ffc0-11e8-92cd-d6efd43419f9.png)

Commits
-------

e54e219 [VarDumper] Use hyperlinks in CliDescriptor
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fabpot fabpot mentioned this pull request May 9, 2019
@danielchodusov
Copy link

Not sure If it's a console component bug .. I'm using iTerm2 and after upgrading to 4.3-Beta I realised debug:router now returns link in the path output. But for some reason, they are not parsed.

Screenshot 2019-05-12 at 14 18 26

@ostrolucky
Copy link
Contributor Author

@danielchodusov can you create issue instead please?

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

Successfully merging this pull request may close these issues.

7 participants