Skip to content

Extend comments-time-machine-links feature#1863

Merged
fregante merged 10 commits intomasterfrom
time-machine-links-for-links
Apr 20, 2019
Merged

Extend comments-time-machine-links feature#1863
fregante merged 10 commits intomasterfrom
time-machine-links-for-links

Conversation

@fregante
Copy link
Member

@fregante fregante commented Mar 18, 2019 •
edited
Loading

I think that the most clear and less troublesome way to solve this is to add an icon:

  • It avoids the "What you see is not what I get issue that'd we'd have if we change links without leaving a visual cue.
  • It clearly shows that a link can be refined, rather than hiding the action behind a button (which I wouldn't know where to place anyway)

a

b

Notes

Tests

And more links in:

Possible bug, should we disable this feature in code blocks?

The clipping bug is actually due to the #1146 and also appears for linkified issues: no full solution, but moving the tooltip to the right might solve it for short links

Suggest more conversations with links like that

Closes #1448

@fregante
Copy link
Member Author

fregante commented Mar 18, 2019

Screenshot 2019-03-19 at 01 41 32

This also updates the old links, however I kinda want to move them to the comment dropdown at some point.

@sindresorhus
Copy link
Member

sindresorhus commented Mar 27, 2019

This looks really useful!

@sindresorhus
Copy link
Member

sindresorhus commented Mar 27, 2019

All links point to the default branch instead of the linked branch because

However, this limitation considerably limits its usability. Is there any way we can do API calls to support getting the correct branch too?

@sindresorhus
Copy link
Member

sindresorhus commented Mar 27, 2019

/source/libs/api.js@master#L3

This test (The second under the Tests header) doesn't work.

@sindresorhus
Copy link
Member

sindresorhus commented Mar 27, 2019

however I kinda want to move them to the comment dropdown at some point.

👍

@sindresorhus
Copy link
Member

sindresorhus commented Mar 27, 2019

should we disable this feature in code blocks?

Yes

@fregante
Copy link
Member Author

fregante commented Mar 27, 2019 •
edited
Loading

Is there any way we can do API calls to support getting the correct branch too?

Yes... if we must. I can test if all those /tree/name are tags. If they are, the icon isn't needed at all because tags are already semi-permalinks

This test (The second under the Tests header) doesn't work.

It does, but it was a bad test since that file didn't exist when I posted my comment, so the link is still a 404. D'oh. I changed it.

@fregante
Copy link
Member Author

fregante commented Apr 18, 2019

However, this limitation considerably limits its usability. Is there any way we can do API calls to support getting the correct branch too?

Too involved, I tried but I'm not interested, best left as a separate issue

should we disable this feature in code blocks?

Done

however I kinda want to move them to the comment dropdown at some point.

Done

Screenshot 2019-04-18 at 15 33 22

I think this only needs docs now

@sindresorhus
Copy link
Member

sindresorhus commented Apr 18, 2019

I tried but I'm not interested, best left as a separate issue

Can you open an issue for it?

@fregante
Copy link
Member Author

fregante commented Apr 18, 2019

Can you open an issue for it?

Once this is merged, as it depends on it

sindresorhus
sindresorhus approved these changes Apr 18, 2019
View reviewed changes
Copy link
Member

@sindresorhus sindresorhus left a comment

Choose a reason for hiding this comment

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

Good to merge when the docs updates are done 👌

@fregante fregante merged commit 49f57be into master Apr 20, 2019
@fregante fregante deleted the time-machine-links-for-links branch April 20, 2019 03:10
@fregante fregante mentioned this pull request Apr 20, 2019
Closed
@fregante fregante changed the title Add timemachine links to all repo links Extend comments-time-machine-links feature Apr 20, 2019
@fregante fregante mentioned this pull request May 18, 2019
Closed
@fregante fregante mentioned this pull request Jun 7, 2019
Closed
@fregante fregante mentioned this pull request May 25, 2020
Merged
@fregante fregante mentioned this pull request Nov 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sindresorhus sindresorhus sindresorhus approved these changes

Assignees

No one assigned

Labels

enhancement

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Permalinks for volatile links in comments

2 participants

@fregante @sindresorhus