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

Minor English improvements + notes below #2747

Merged
merged 1 commit into from
Jun 23, 2020

Conversation

mark-summerfield
Copy link
Contributor

  1. I've replaced "Aka" with "alias". If that's not wanted I still think "Aka" is wrong and should be replaced by something else.
  2. On lines 549-550 it says you can write a named character entity like this: \name. But I tried doing: auto x = "\euro"; and it gave an error.
  3. It is a real pity that __DATE__ uses a US-specific format rather than the universal ISO8601. I hope an __ISODATE__ and an __ISOTIMESTAMP__ are added to repair this.
  4. I don't know what the explanation for __EOF__ means. Would "Tells the scanner to stop at this point in the source." be a correct alternative?
  5. This page's very last example has the string "foo\bar". Shouldn't this be r"foo\bar" or "foo\\bar"? And if not, a reason ought to be given.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @mark-summerfield! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@thewilsonator
Copy link
Contributor

  1. While those are technically aliases they are they are almost never referred to as the underlying symbol, even by the compiler. e.g. if the type immutable(char)[] were to be part of an error message it would be displayed as string, so I think aka is fine there.

  2. "Tells the lexer to stop at this point in the source."

  3. Most probably. \b is backspace and doesn't make much sense in a string.

@mark-summerfield
Copy link
Contributor Author

I still think use of "Aka" is poor practice. It is an abbreviation and not necessarily well known to non-native English speakers.

spec/lex.dd Show resolved Hide resolved
spec/lex.dd Outdated Show resolved Hide resolved
@Geod24
Copy link
Member

Geod24 commented Feb 3, 2020

On lines 549-550 it says you can write a named character entity like this: \name. But I tried doing: auto x = "\euro"; and it gave an error.

The correct syntax is: auto x = "\€";

It is a real pity that __DATE__ uses a US-specific format

That's a very valid point. Although CTFE should allow us to convert from one another, easing the pain, it is not a good default. Could you raise an issue about it ?

Would "Tells the scanner to stop at this point in the source." be a correct alternative?

Yes, or "Ignore everything after this token".

This page's very last example [...]

Looks like it's an error, yes. The correct string would be "foo\bar", but I think "foo/bar.d" would be more adequate, or even "pkg/mod.d".

@Geod24
Copy link
Member

Geod24 commented Feb 3, 2020

And thanks for your work!

spec/lex.dd Outdated Show resolved Hide resolved
@thewilsonator
Copy link
Contributor

Hmm, that still seems to fail. Maybe take that out and verify that is the case of the problem and do it in another PR?

@thewilsonator
Copy link
Contributor

Optimistically adding auto-merge

@thewilsonator
Copy link
Contributor

Hmm it failed, but not for the same reason:

http://downloads.dlang.org/releases/2.x/2.088.0/dmd.2.088.0.linux.tar.xz | tar -C ../generated/host_dmd-2.088.0 -Jxf - || rm -rf ../generated/host_dmd-2.088.0
../generated/host_dmd-2.088.0/dmd2/linux/bin64/dmd -conf=../generated/host_dmd-2.088.0/dmd2/linux/bin64/dmd.conf -of../generated/build build.d
../generated/build HOST_DMD="../generated/host_dmd-2.088.0/dmd2/linux/bin64/dmd" CXX="c++" OS=linux BUILD=release MODEL=64 AUTO_BOOTSTRAP="1" DOCDIR="" STDDOC="" DOC_OUTPUT_DIR="" MAKE="make" --called-from-make dmd
posix.mak:110: recipe for target 'dmd' failed
make[1]: *** [dmd] Terminated
posix.mak:611: recipe for target '.generated/dmd-2.090.0/generated/linux/release/64/dmd' failed
make: *** [.generated/dmd-2.090.0/generated/linux/release/64/dmd] Terminated
fatal: 'refs/ae-sys-d-cache/website-97da142fc48e01b0d2fbaf6055a95a21a4de9cb6-fc6a263ed9b6802d2e6ac01308d4159d' - not a valid ref

I'm not sure where that failed. @CyberShadow any guesses?

@CyberShadow
Copy link
Member

Something seems to have caused the host compiler to terminate while building the master compiler. I don't know what or why. Unless master is broken (which it doesn't seem to be), it is most likely a random failure (due to PID reuse or something of a similar nature). Should go away on a retry (rebase/amend and force-push).

1. I've replaced "Aka" with "alias". If that's not wanted I still think "Aka" is wrong and should be replaced by something else.
2. On lines 549-550 it says you can write a named character entity like this: `\name`. But I tried doing: `auto x = "\euro";` and it gave an error.
3. It is a real pity that `__DATE__` uses a US-specific format rather than the universal ISO8601. I hope an `__ISODATE__` and an `__ISOTIMESTAMP__` are added to repair this.
4. I don't know what the explanation for `__EOF__` means. Would "Tells the scanner to stop at this point in the source." be a correct alternative?
5. This page's very last example has the string `"foo\bar"`. Shouldn't this be `r"foo\bar"` or `"foo\\bar"`? And if not, a reason ought to be given.
@Geod24
Copy link
Member

Geod24 commented Jun 23, 2020

Rebased on master, squashed the commits together, and removed a trailing whitespace on L361 of lex.d.

@dlang-bot dlang-bot merged commit e099b26 into dlang:master Jun 23, 2020
@mark-summerfield mark-summerfield deleted the patch-2 branch June 23, 2020 07:25
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.

6 participants