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

Rewrite best practices to encourage tabs #74

Closed
wants to merge 4 commits into from
Closed

Rewrite best practices to encourage tabs #74

wants to merge 4 commits into from

Conversation

hexylena
Copy link
Member

While still allowing spaces, and just promoting consistency. Fixes #73

While still allowing spaces, and just promoting consistency.
Fixes #73
@nsoranzo
Copy link
Member

I am heavily conflicted on this. I fully support the goal of improving accessibility, but I don't know enough about the available tools to say this is the best solution.
I think we should explicitly exclude Python scripts from this recommendation, given that PEP-8, black, standard library and all major Python projects (including the Galaxy ecosystem) all use 4-space indentation. Or specify it applies only to the XML files.

@hexylena
Copy link
Member Author

Yes, agreed. As written, both of these changes only affects tool XML related sections.

One is specifically in the docs/best_practices/tool_xml.rst, while the other is the last bullet of a heading Creating the Tool Wrapper (XML File)

@peterjc
Copy link
Contributor

peterjc commented Nov 14, 2022

As @hexylena points out this is under an XML section, but would making the new bullet point starting "Use tabs for indentation" read "Use tabs for XML indentation" help @nsoranzo?

@nsoranzo
Copy link
Member

Yes, agreed. As written, both of these changes only affects tool XML related sections.

One is specifically in the docs/best_practices/tool_xml.rst, while the other is the last bullet of a heading Creating the Tool Wrapper (XML File)

The Coding Style section in docs/best_practices/tool_xml.rst talks also about Python code style later on, so I think it could be confusing there.

"Tabs for XML indentation" would work for me!

@hexylena
Copy link
Member Author

Fixed, per y'all's suggestion :) @nsoranzo @peterjc

@bgruening
Copy link
Member

"Tabs for XML indentation"

This means only the XML tags inside the XML files, so not the cheetah and the rst section?

@peterjc
Copy link
Contributor

peterjc commented Nov 14, 2022

Good question, can the cheetah and RST be done with either tabs or space indentation from a technical point of view?

The RST specification says at https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#whitespace that:

Spaces are recommended for indentation, but tabs may also be used.

It may be pragmatic to recommend tabs only for the XML tags, and spaces for embedded RST?

@hexylena
Copy link
Member Author

hexylena commented Dec 5, 2022

I'd like to see it recommended for the cheetah sections as well (that's always worked fine) and since by their documentation it works fine for rST, I'd argue we should provide a more consistent guidance, namely:

  • tabs are suggested for all parts, the XML, the cheetah (works), and the rST (supported!)
  • and if you are consistent, you are permitted to use spaces for all three
  • we do not recommend mixing tabs and spaces in the file, consistency is encouraged

@bgruening
Copy link
Member

I think we are pretty much a Python community (cheetah, rst ...) and they recommend spaces instead of tabs. I don't see why we should recommend tabs here.

Maybe we can remove the current recommendation and just not recommend anything, just staying stay consistent. Overall the important part is that people can use what they can deal with best.

@hexylena
Copy link
Member Author

hexylena commented Dec 5, 2022

I don't see why we should recommend tabs here.

This discussion in a large part was started by wanting to make things more accessible to everyone, and more welcoming for those users, hence making that the recommendation, but allowing it open to personal choice.

and just not recommend anything, just staying stay consistent

As it stands, with the text as it is now, any contributor would be free to stick with spaces. But by having the recommendation we can at least push for some small tiny amount of slow change in a positive direction.

The only time having this "recommendation" would be bad, is if a reviewer decided to ignore that it is a "recommendation" and make it mandatory for a contributor.

@hexylena
Copy link
Member Author

hexylena commented Dec 5, 2024

given the lack of motion on this, I will close it.

@hexylena hexylena closed this Dec 5, 2024
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.

Tabs should be permitted, maybe even mandatory.
4 participants