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

fixed #138 #147

Closed
wants to merge 1 commit into from
Closed

fixed #138 #147

wants to merge 1 commit into from

Conversation

VladimirAlexiev
Copy link
Contributor

@VladimirAlexiev VladimirAlexiev commented Feb 26, 2023

I took up @TallTed's suggestion about rule formatting.
But I included some more formatting fixes. Hope I haven't overdone it, and the PR can be accepted!


Preview | Diff

@william-vw
Copy link
Collaborator

Many of these don't look like fixes, but rather like personal preferences on the use of spaces and newlines. There's no best practices on their use in N3, as long as indents aren't misleading (e.g., as to what property belongs to which resource). Thus, I'd opt not to merge this PR, unless a case can be made that the fixes, well, actually fix something.

@TallTed
Copy link
Member

TallTed commented Feb 27, 2023

unless a case can be made that the fixes, well, actually fix something

My suggested tweaks to indentation and other whitespace are primarily made on examples which are provided for humans to read, not for machines to digest. @VladimirAlexiev's appear to be in the same vein. Thus, they don't "fix" anything in the sense I think you're asking for, but they certainly increase comprehension, in my experience, so... if that's something you value in this documentation for humans, I would suggest this be merged.

@william-vw
Copy link
Collaborator

@TallTed ... I think we all agree the goal is having examples for human consumption - if it wasn't, all of these would be minified one-line examples with only the minimal whitespace required by the grammar.

The PR is represented as a set of formatting fixes, so my question pertains to relevant formatting principles. These would be useful for other purposes as well, such as the automatic formatting of N3 code. In general, formatting of N3/Turtle code for readability is tricky - Jena applies a number of heuristics for it (e.g., depending on the length of predicates, number of object values and their length, ...). Here, the proposed updates seem rather arbitrarily decided.

Whatever is decided on the formatting of these examples, the benefit of particular principles should be well motivated.

@TallTed
Copy link
Member

TallTed commented Feb 28, 2023

@william-vw

I agree, it would be better if the guidelines of "human friendlier" formatting could be clearly stated and therefore followed by any and all.

And, on further review of this PR from @VladimirAlexiev, I disagree with many if not most of his proposed changes (a frequent change I notice is @VladimirAlexiev removing spaces between enclosing braces and the data enclosed therein, e.g., he's changing { :a :b :c } to {:a :b :c}, which I would not do because this can make it seem that the enclosed content is attached to the enclosing braces; my general practice is to increase optional whitespace for human-targeted data, though it may be reduced or removed for machine-targeted data).

So I guess I'm asking @VladimirAlexiev and you and any others who are interested, to participate with me in an Issue discussing such things (which I don't believe exists yet), before any large-scale changes are put through PR.

@william-vw
Copy link
Collaborator

@TallTed @VladimirAlexiev please feel free to update the relevant issue #138 - or start a new one - with a discussion on rule formatting options. For now, I will close this PR.

@william-vw william-vw closed this Mar 8, 2023
@VladimirAlexiev
Copy link
Contributor Author

I've added proposed "Formatting Rules" to #138 (comment).
@william-vw and @TallTed please comment.

@TallTed

I disagree with many if not most of his proposed changes

I've added your suggestion "Surround punctuation (brackets, parentheses, commas, semicolons, dots) with spaces" even though I prefer no spaces.

Please specify in detail which other changes you disagree with. Thansk!

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.

None yet

3 participants