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

Fix links from $(D $(LREF ...)) -> $(LREF ...) #5242

Merged
merged 1 commit into from
Mar 5, 2017

Conversation

wilzbach
Copy link
Member

@wilzbach wilzbach commented Mar 5, 2017

From the NG

On second thought, yes, ddox shouldn't output invalid HTML if it can help it, but this is actually an antipattern I see a lot in Phobos and hate:
$(D $(LREF Abort)))
That's an unnecessary repetition! If it is an LREF in a D source file, then you already know it is a D symbol, so the $(D) macro is redundant anyway.
I'd simplify that to just be $(LREF ...) in all cases it appears in the Phobos source, and then if you want it to be styled in the monospace font, do a css rule for that.

Your wish has been heard:

sed -E 's/[$]\(D [$]\(LREF (.*)\)\)/$(LREF \1)/' -i **/*.d

CC @andralex @adamdruppe

@wilzbach
Copy link
Member Author

wilzbach commented Mar 5, 2017

Should we add a check like [$]\(D [$]\(LREF .*\)\) to prevent this anti-pattern for all time?

sed -E 's/[$]\(D [$]\(LREF (.*)\)\)/$(LREF \1)/' -i **/*.d
@andralex
Copy link
Member

andralex commented Mar 5, 2017

@wilzbach of course, please! Automation is a work divider and a result multiplier. Thanks!

Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

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

awesome possum

@andralex
Copy link
Member

andralex commented Mar 5, 2017

(why no auto-merge-squash in this repo?)

@dlang-bot dlang-bot merged commit 0b41c99 into dlang:master Mar 5, 2017
@adamdruppe
Copy link
Contributor

good stuff!

another thing i've love to do is to eventually remove ALL the _symbolname things... but that's not a simple sed call.

$(TD Joins a couple of functions into one that executes the original
functions independently and returns a tuple with all the results.
))
$(TR $(TD $(D $(LREF compose)), $(D $(LREF pipe)))
$(TR $(TD $(LREF compose)), $(LREF pipe)
Copy link
Contributor

Choose a reason for hiding this comment

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

wait a minute this looks wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be $(TR $(TD $(LREF compose), $(LREF pipe)). The regular expression didn't handle nested parenthesis correctly and now the second link will show up outside the table cell.

$(TD Forwards function arguments while saving ref-ness.
))
$(TR $(TD $(D $(LREF lessThan)), $(D $(LREF greaterThan)), $(D $(LREF equalTo)))
$(TR $(TD $(LREF lessThan)), $(LREF greaterThan)), $(D $(LREF equalTo)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is wrong too.

$(TD Creates a function that binds the first argument of a given function
to a given value.
))
$(TR $(TD $(D $(LREF reverseArgs)), $(D $(LREF binaryReverseArgs)))
$(TR $(TD $(LREF reverseArgs)), $(LREF binaryReverseArgs)
Copy link
Contributor

Choose a reason for hiding this comment

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

And another. Methinks regex is too greedy for this. Relatively small set to fix up by hand though (I just did it for the merge conflict on my adrdox fork, alas I can't do a PR straight off that due to the myriad other differences that have accumulated there over the last year).

$(TD Converts a callable to a delegate.
))
$(TR $(TD $(D $(LREF unaryFun)), $(D $(LREF binaryFun)))
$(TR $(TD $(LREF unaryFun)), $(LREF binaryFun)
Copy link
Contributor

Choose a reason for hiding this comment

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

you get hte idea i won't comment every oen

@adamdruppe
Copy link
Contributor

I guess none of these are a simple sed call since regex doesn't handle paren matching very well. But I g2g, church time and the whole day is fairly busy. I'll be back on email later in the evening though.

@wilzbach wilzbach deleted the fix-lref-links branch March 5, 2017 16:40
@wilzbach
Copy link
Member Author

wilzbach commented Mar 5, 2017

(why no auto-merge-squash in this repo?)

Martin killed it yesterday: dlang/dlang-bot#64

I guess none of these are a simple sed call since regex doesn't handle paren matching very well.

Yeah it should have been sth. like, but it was way too late for me :/

sed -E 's/[$]\(D [$]\(LREF ([^)]*)\)\)/$(LREF \1)/' -i **/*.d

@wilzbach
Copy link
Member Author

wilzbach commented Mar 5, 2017

another thing i've love to do is to eventually remove ALL the _symbolname things... but that's not a simple sed call.

I have a couple of replacement script based on libdparse lying around, so it shouldn't be too difficult to make that work. However, we would need to fix DDoc first as those _symbolname are hacks to work around DDocs feature of auto-recognizing the function/module name in the documentation.

@adamdruppe
Copy link
Contributor

adamdruppe commented Mar 5, 2017 via email

@wilzbach
Copy link
Member Author

wilzbach commented Mar 5, 2017

in the macro file. Defining it out of existence means even if it is triggered,
it just displays the original text to begin with. Then we transition
Phobos,

I was more referring to the fact that there needs to be a consensus / agreement that we get rid off this "feature". Do you care enough to make a short post on the NG about it?

@adamdruppe
Copy link
Contributor

http://forum.dlang.org/post/[email protected]

I think we should just do it anyway though, Phobos already pretty consistently uses the underscore EVERYWHERE, indicating that the authors nearly every time want to turn the behavior off, and I doubt anyone will come in and say "yeah that's an awesome feature that I want more often than not".

Besides, we don't need approval from the community to kill it in Phobos - dlang.org and phobos are free to define its own macro set anyway!

But I posted since I am genuinely curious if ANYONE finds it useful.

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.

4 participants