Skip to content
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

Enhance view-markdown-source with line actions #3594

Merged
merged 10 commits into from
Sep 27, 2020

Conversation

yakov116
Copy link
Member

@yakov116 yakov116 commented Sep 25, 2020

@yakov116 yakov116 marked this pull request as ready for review September 25, 2020 14:17
@yakov116 yakov116 requested a review from fregante September 27, 2020 00:50
Copy link
Member

@fregante fregante left a comment

Choose a reason for hiding this comment

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

Other than that, LGTM, merge at will.

yakov116 and others added 3 commits September 26, 2020 22:48
Co-authored-by: Federico <me@fregante.com>
Co-authored-by: Federico <me@fregante.com>
I will make a lint pr to sort them all
@yakov116 yakov116 merged commit f611eae into refined-github:master Sep 27, 2020
@yakov116 yakov116 deleted the makrdown branch September 27, 2020 12:25
@yakov116
Copy link
Member Author

@fregante great job! (It's all your code 😀)

kidonng pushed a commit to kidonng/refined-github that referenced this pull request Oct 7, 2020
Co-authored-by: Fregante <opensource@bfred.it>
@fregante
Copy link
Member

I'd like to blame my code because this is a bad idea. Don't we already have access to this code via the page we're loading?

Currently all unrelated code is being stripped by the second parameter of

const dom = await fetchDom(path, '.blob-wrapper');

but I think that we can avoid having to maintain these 70 lines by using something like:

const page = await fetchDom(path);
lineActions /*global*/ = select('blah blah', page);
const source = select('.blob-wrapper', page);

I don't think we even need to customize anything in lineActions since it comes straight from GitHub

@yakov116
Copy link
Member Author

I can give it a shot. This should be part of November lint 🙄

@yakov116
Copy link
Member Author

yakov116 commented Oct 19, 2020

Don't we already have access to this code via the page we're loading?

Did some debugging. We dont, it looks like its loaded loader on

@fregante
Copy link
Member

Indeed. We fetch the blame page but only the regular source page is supposed to have it, like https://github.com/sindresorhus/refined-github/blob/master/.editorconfig#L3

I've noticed this problem because this feature is currently the longest file and to be fair it's a nightmare just looking at it imagining what class will be dropped next by GitHub.

I wonder if we could just fetch it from a random file:

const lineActions = onetime(() => {
	const bar = fetchDom('https://github.com/sindresorhus/refined-github/blob/b1229bbaeb8cf071f0711bc2ed1b40dd96cd7a05/.editorconfig', 'BlobToolbar');
	select('#js-view-git-blame', bar)!.href = new GitHubURL(location.href).assign({route: 'blame'}).href;
	select('#js-new-issue', bar)!.href = `/${getRepoURL()}/issues/new`;
	return bar;
});

We'd need a !isEnterprise but the maintainability improvement is worth it.

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

Successfully merging this pull request may close these issues.

Enhance view-markdown-source with line actions
2 participants