-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 unified 'sphinx' command (WIP!) #6938
base: master
Are you sure you want to change the base?
Add unified 'sphinx' command (WIP!) #6938
Conversation
Let argparse do the heavy lifting here for us. Signed-off-by: Stephen Finucane <stephen@that.guru>
@tk0miya I've only ported two of the four commands currently available in tree as part of this PR. Before I work on the remainder, I have a few questions I'd like your input on.
|
Use argparse's built-in functionality to handle these as part of the parsing process itself, rather than as an afterthought. This changes behavior somewhat, as we'll now fail on the first missing file rather than list all missing files, but that doesn't seem like such a bad thing. Signed-off-by: Stephen Finucane <stephen@that.guru>
Once again, this moves the logic into argparse and away from the meat of our application. Signed-off-by: Stephen Finucane <stephen@that.guru>
This will form the basis of a future 'sphinx' executable and subcommands. Signed-off-by: Stephen Finucane <stephen@that.guru>
This will make an upcoming diff far easier to read. Signed-off-by: Stephen Finucane <stephen@that.guru>
This is the same as 'sphinx-quickstart' and represent the first step towards a unified executable. Note that this might look like a big diff, but most of it is just indentation due to the use of the class. Signed-off-by: Stephen Finucane <stephen@that.guru>
As seen previously, this will make an upcoming diff far easier to read. Signed-off-by: Stephen Finucane <stephen@that.guru>
This is the same as 'sphinx-build' and represent the second step towards a unified executable. Signed-off-by: Stephen Finucane <stephen@that.guru>
25434be
to
5e33716
Compare
As noted at #6939 (comment), another thing we could do here in change the behavior of the outputdir and maybe the sourcedir options for
to
This would mean we'd need to be able to define
|
Yes, of course! I just read a part of this patch yet. There might be trivial comments and requests later. But it looks good to me. I'll continue to read this later (maybe my new years task!).
+1. new sphinx command should not follow historical interfaces. We can forget both make and non-make mode! |
Good to hear. I am holding off on this until I determine whether I can configure the output directory as an optional flag (so |
|
||
return parser | ||
|
||
def run(self, argv: List[str]) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please call initialization of locales at beginning of the application.
sphinx.locale.setlocale(locale.LC_ALL, '')
sphinx.locale.init_console(os.path.join(package_dir, 'locale'), 'sphinx')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
except ValueError: | ||
pass | ||
confoverrides['html_context.%s' % key] = val | ||
return BuildCommand._run(args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to discuss sphinx build
(new stye) is same as sphinx-build
(old style). I still don't have an answer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only behavior change I've suggested so far is to go from:
sphinx-build sourcedir builddir [file, ...]
to
sphinx build sourcedir [file, ...] [-o builddir]
But perhaps there are other options that we think are badly named and should be renamed now? I hadn't thought of that so good idea 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just an idea, sphinx build html
is so so good to me.
help=__('quiet mode')) | ||
parser.add_argument('--version', action='version', dest='show_version', | ||
version='%%(prog)s %s' % __display_version__) | ||
'sphinx init' is an interactive tool that asks some questions about your |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some tools, init
subcommand goes project initialization silently. So I wonder we should separate quickstart feature into sphinx init
and sphinx quickstart
(?) (or sphinx init --interactive
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you thinking of git init
? I think it's okay for sphinx init
to be interactive, so long as you can specify these options from the command line and avoid the interactive prompts. We do need some way to retrieve required information like project
and author
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was thinking of git init
and bundle init
. But I found npm init
asks questions interactively. So please forget my comment.
We do need some way to retrieve required information like project and author.
Actually, project
and author
are not required configuration. So we can suppress all of questions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually,
project
andauthor
are not required configuration. So we can suppress all of questions.
Oh, I didn't know that. Good to know. However, is it really useful to ignore these? What advantage would sphinx init
provide over touch conf.py
if we didn't ask for these? I think it makes sense to ask for these at a minimum, even if people can skip past them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems some users would like to do interactions more verbosely (see #6656). I still don't have an answer for it. But, as one of idea, it might be better to provide both silent and verbose way.
Note: silent means only "less". It does not equal to no question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay. How about we make interactive not the default by adding an --interactive
flag? With this flag, we would request all configuration options in an interactive manner from the user, but users that don't want to do this can simply pass the relevant args they care about, e.g. --project
? That would satisfy the request of #6656 while not annoying people like you that just wants to start a really quick project? The other alternative is a combination of --quick
(default) and --full
arguments, with the latter enabling configuration of extensions etc. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer an option like --full
. It is too much to choose non interactive mode by default to me. But it is annoying to me to list all configurations. I think it is difficult to choose to appropriate options or extensions for standard users. So it would be better to ask minimum configurations by default.
Feature or Bugfix
Purpose
Rather than having multiple separate executables -
sphinx-build
,sphinx-quickstart
, etc. - add a singlesphinx
command with multiplesubcommands.
Detail
TODO.
Relates