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

Add codeberg link parsing #1194

Merged
merged 1 commit into from
Oct 6, 2022
Merged

Add codeberg link parsing #1194

merged 1 commit into from
Oct 6, 2022

Conversation

p00f
Copy link
Contributor

@p00f p00f commented Sep 16, 2022

No description provided.

@p00f
Copy link
Contributor Author

p00f commented Oct 6, 2022

@dandavison can you take a look? Codeberg links are the same format as GitHub so there isn't much change

@dandavison
Copy link
Owner

dandavison commented Oct 6, 2022

Hi @p00f, thanks and sorry for being unresponsive! Can you send me shell commands to clone a codeberg repo so that I can try this out?

I think that what I'd ideally like is to add the ability for users to configure the remote link parsing regex themselves. (Is there any chance you feel like working on that??) That was what made me hesitate here. It is true that I have only barely heard of codeberg (or sourcehut), but of course I don't want to privilege the well-known git hosting companies over smaller ones. Nevertheless I would ideally like a more scalable solution that doesn't involve delta hard-coding this logic, even if, in practice, I am not about to be inundated with thousands of pull requests adding regular expressions parsing URLs from the world's lesser-known git hosting companies :)

@p00f
Copy link
Contributor Author

p00f commented Oct 6, 2022

sure, git clone https://codeberg.org/dnkl/fuzzel

That was what made me hesitate here

I was about to write "don't worry, I don't think there are other FOSS-focused hosts" 😄

Is there any chance you feel like working on that??

Yes! (Can the "configuration" for the major ones i.e. sourcehut and codeberg be built-in along with github and gitlab?)

How would I write tests for this?

@dandavison dandavison merged commit ae4f774 into dandavison:master Oct 6, 2022
@dandavison
Copy link
Owner

OK, merged this, thanks very much!

(Can the "configuration" for the major ones i.e. sourcehut and codeberg be built-in along with github and gitlab?)

Sure. But, actually, I wonder whether maybe this is more work than it's worth. Can you think of a good configuration interface/API for users to specify this parsing logic? We have --file-transformation where users supply a sed-style transformation, but it would need to be more complicated than that. I'm thinking perhaps there are more important things to work on in delta (e.g. line wrapping in the non-side-by-side cases #1176 #299)

@p00f
Copy link
Contributor Author

p00f commented Oct 6, 2022

In #1176 and #299 the issue is that blame and non-side-by-side are not wrapped, correct?

@dandavison
Copy link
Owner

That's right. @th1000s implemented word-wrapping for the side-by-side case but we don't yet use any of that wrapping machinery in non-side-by-side contexts.

@p00f p00f deleted the codeberg branch October 7, 2022 10:10
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.

2 participants