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 folder_include_patterns and file_include_patterns #129

Open
wants to merge 8 commits into
base: sublime-text-3
Choose a base branch
from
Open

Add folder_include_patterns and file_include_patterns #129

wants to merge 8 commits into from

Conversation

ferhtgoldaraz
Copy link

Extend the documentation of project folder declaration explaining a bit further it's optional values, and adding two that were not being mentioned

Please note, english is not my first language. It would be appreciated if you could read and ammend the changes if they happen to sound weird or something.

Extend the documentation of project folder declaration explaining a bit further it's optional values, and adding two that were not being mentioned

Please note, english is not my first language. It would be appreciated if you could read and ammend the changes if they happen to sound weird or something.
and ``file_exclude_patterns``. They all admit an array of strings
as values, and understand basic globbing syntax, such as "*.html".

The ones that include files and folders, work by excluding
Copy link
Collaborator

Choose a reason for hiding this comment

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

You are referring to the "patterns" here, not the settings. While it can be interpreted to be the patterns, explicitly mentioning it is better. (It confused me at first.)
Same for the second part of this paragraph.

@FichteFoll
Copy link
Collaborator

Thanks for your contribution, I added a few comments.

Please try to restructure your additions to use semantic linefeeds as mentioned here.

As per some comments made, I've tried my best to make the explacantion a bit clearer, and added the suggested edits.
@ferhtgoldaraz
Copy link
Author

Thanks for your input, I've tried my best to improve the explanation and to update the style. I've done what I can with semantic line feeds, but I might have not got it, so feel free to reformat at will

@ferhtgoldaraz
Copy link
Author

There are some file references written as foo that are not working. I'm gonna leave tham as foo for now, as :file:foo`` doesn't seem to work. I'm afraid I'm not familiar with that syntax

@FichteFoll
Copy link
Collaborator

The file role is define in Sphinx here. I'm not sure where you are seeing that the file role wouldn't work because it's being used throughout the code base (for example here).
Either way, just foo would be worse since restructured text requires double backticks for inline literals, that is foo.

I will take a look at your changes in detail later.

Edit: It appears that it will probably easier if I just take whatever you have here (by squashing probably) and then do my own pass.

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.

2 participants