doc: rename "recursive search" to "directory scan" #2559

shuber2 · 2022-11-04T21:12:39Z

The method 6 of finding the main file is based on a directory scan which is described as "recursive" scan. However, while a recursive scan suggests a (recursive) directory descent, the opposite is true: the unique ascending path of parent directories is scanned.

This is fixed in the following steps:

The method is renamed to "directory scan method"
The method description is adapted to reflect this behavior more clearly. In particular, it is explicitly mentioned that no recursive directory descent is performed.
The option to disable this method is renamed to g:vimtex_disable_directory_scan_main_file_detection

As a side effect, the overall description of the directory scan method is reworked.

As a further side effect, the name of this method is more independent of the algorithmic mechanism, but rather reflects the policy, cf. "policy versus mechanism" paradigm.

shuber2 · 2022-11-04T21:26:49Z

@lervag Sorry, I was not thinking you would be that quick with the work on the other issue. So this pull request will have merge conflict with #2558.

lervag · 2022-11-05T15:07:37Z

The method 6 of finding the main file is based on a directory scan which is described as "recursive" scan. However, while a recursive scan suggests a (recursive) directory descent, the opposite is true: the unique ascending path of parent directories is scanned.

Interesting point. The reason I use "recursive" here is because it is functionally recursive in the sense that same algorithm is used recursively for complex documents, e.g. main.tex ← chapter.tex ← section.tex. If you open section.tex, it will being the search from itself, then find that chapter.tex contains section.tex. It then recurses and finds main.tex in the next "iteration". Thus, the point of "recursive" is not about the directory descent at all, it has to do with the project graph of a multi-file LaTeX project.

I'm, curious, though, if you may still have a point with regard to user point of view. What I explained above is an implementation detail, and it is not important (in most cases) for users to care about that.

I'm curious what you think about this.

In any case, I notice that your PR also has good suggestions for how to rephrase and improve the wording, also in general. So I will want to merge it somehow, regardless of the outcome of this discussion!

The option to disable this method is renamed to g:vimtex_disable_directory_scan_main_file_detection

If we rename an option, we should put the old option in a deprecated list to guide users. That's trivial for me, so I can do it if we go this way. Or I could show you the relevant part in the code.

As a further side effect, the name of this method is more independent of the algorithmic mechanism, but rather reflects the policy, cf. "policy versus mechanism" paradigm.

Yes, this seems to be relevant to what I wrote above.

Sorry, I was not thinking you would be that quick with the work on the other issue. So this pull request will have merge conflict with #2558.

I have no problems with merge conflicts. It should be mostly trivial in this case, I think. I'll let you decide if you want to rebase your PR on top of the current master, or if you want me to handle the merge when we've settled the discussions.

doc/vimtex.txt

shuber2 · 2022-11-05T20:03:53Z

@lervag Thank you for your feedback. I did some work on this:

Rebase to current master
Rephrasing to address your feedback
Adding deprecation information in logic and doc

I also rephased the description of the directory scan method again to make the point on the recursive expansion clearer, including a reference to g:vimtex_include_indicators.

When it comes to the overview of methods, the essence of method 6 is that directories are scanned to find a suitable main file. In my opinion, the name of the method should therefore reflect this aspect. When we now describe details when a candidate tex file qualifies as a main file, the aspect of parsing it and recursively expanding it through included tex files becomes relevant. But this is a downstream aspect. And it is an aspect that is true for method 1-5 as well, namely checking whether a candidate tex file is a suitable main file for the current file. What distinguishes method 6 from method 1-5 is that a search for a main file is performed, i.e., directories are scanned for such a file.

shuber2 · 2022-11-06T07:17:38Z

A minor rephrasing and a minor spelling fix added.

lervag

Thanks! I think this is going in a good direction that should help make things clearer! I much appreciate your efforts, and I hope you don't find my feedback too nitpicky.

lervag · 2022-11-06T14:22:38Z

autoload/vimtex/state.vim

@@ -237,7 +237,7 @@ function! s:get_main() abort " {{{1
  endif

  " Search for main file recursively through include specifiers
-  if !get(g:, 'vimtex_disable_recursive_main_file_detection', 0)
+  if !get(g:, 'vimtex_disable_directory_scan_main_file_detection', 0)


I'm actually considering if we should instead just remove this option alltogether. I don't think anyone uses it. If someone complained, it would be a useful feedback in itself, and I could revert that specific change and use the name you suggest here.

One would like to disable it for performance reasons. The feedback on #2558 seems that suggest that this is generally not an issue, but it is hard to estimate for all actual or potential users.
I will implement the removal as a dedicated commit, so it can be cleanly reverted.

lervag · 2022-11-06T14:22:40Z

autoload/vimtex/state.vim

@@ -394,7 +394,7 @@ function! s:get_main_recurse(...) abort " {{{1
  let l:re_filter1 = fnamemodify(l:file, ':t:r')
  let l:re_filter2 = g:vimtex#re#tex_input . '\s*\f*' . l:re_filter1

-  " Search through candidates found recursively upwards in the directory tree
+  " Search through candidates found upwards in the directory tree
  let l:results = []
  for l:cand in s:findfiles_recursive('*.tex', fnamemodify(l:file, ':p:h'))


Perhaps we should rename s:findfiles_recursive here? And possibly the s:get_main_recurse as well? It seems to be in line with your suggestions here.

Or, perhaps the s:get_main_recurse is ok, since it is used recursively. But s:findfiles_recursive could perhaps be better named as s:findfiles_directory_scan?

Had the same thoughts. But I think this should be a separate commit to make debugging with git bisect easier later. I will work on that!

lervag · 2022-11-06T14:33:42Z

doc/vimtex.txt

+There are several alternative methods for specifying the main file that can be
+more flexible and are relevant for certain work flows and use cases. These
+methods all require some explicit declaration of the main file and are
+therefore tried prior the directory scan.


prior to the?

lervag · 2022-11-06T14:34:25Z

doc/vimtex.txt

-  search detects the main LaTeX file by searching for a file in the current
-  and parent directories that
+Directory scan~
+  If none of the above methods gives an appropriate candidate for a main file


gives -> give

"for a main file that directly or indirectly includes the present file"

Or, perhaps simply: "If none of the above methods yield an appropriate main file, then a search ..."?

I think "gives" is ok because the indivual, single methods are meant; it can be read as "none of the [individual] above mehtods gives". It would need to be "give" if phrased as "If the above methods give no appropriate…".

By the way, it is "If x then y", not "If x, then y". I saw the comma somewhere else arleady in the docs I think.

I think "gives" is ok because the indivual, single methods are meant; it can be read as "none of the [individual] above mehtods gives". It would need to be "give" if phrased as "If the above methods give no appropriate…".

Ok!

By the way, it is "If x then y", not "If x, then y". I saw the comma somewhere else arleady in the docs I think.

This is an example of a conditional subordinate conjunction, and I think you need the comma. I found a couple of semi-good references in a quick search:

https://english.stackexchange.com/questions/373905/how-do-you-punctuate-an-if-then-when-both-the-if-and-the-then-are-implied

https://www.ego4u.com/en/cram-up/writing/comma?10

lervag · 2022-11-06T14:37:35Z

doc/vimtex.txt

-  3. contains `\begin{document}` in its parsed content.
+  To test whether a candidate tex file qualifies as a main file the following
+  happens: First, the candidate tex file is parsed and recursively expanded by
+  all tex files included. Then it is checked whether all of the three


the three "following" requirements

yes, makes sense.

lervag · 2022-11-06T14:39:57Z

doc/vimtex.txt

+  Note that no recursive directory descents are performed to find a main file.
+  That is, if the current file is `./B/chapter.tex` then `./A/main.tex` will not
+  be found as the main file, because the descent to subdirectory `./A/` is not
+  performed.


Just curious: directory descent is when we go into subdirectories, right? And directory ascent goes towards the root?

Yes, ascent resp. descent in the directory tree, and (rooted) trees are typically drawn from the root downwards.

I think that's what confuses me. A tree is naturally pointed upwards, but in these cases they are used downwards. 🤷🏻

By "naturally" you mean biological trees? Yes. In computer science, however, trees are drawn with the root at the top, see https://en.wikipedia.org/wiki/Tree_(data_structure)

Or consider the tree shell command or file browsers.

lervag · 2022-11-06T14:40:42Z

doc/vimtex.txt

+  performed.
+
+  In cases where the directory scan fails, one may instead rely on the TeX
+  root directive as an alternative.


Perhaps we should just refer to any of the methods 1-5, instead of only the tex root alternative?

That is true, well spotted.

lervag · 2022-11-06T14:40:56Z

doc/vimtex.txt


  Note: In rare cases, such as if there are _very_ many tex files in the
        directory tree, this method may be slow. One may therefore disable it
-        through the option |g:vimtex_disable_recursive_main_file_detection|.
+        by the option |g:vimtex_disable_directory_scan_main_file_detection|.


I think we can remove this note and the option, as discussed earlier.

See comment above. I will do that in a separate commit, so it can be atomically reverted.

lervag · 2022-11-06T14:41:06Z

doc/vimtex.txt

+  In rare cases, the directory scan method of finding the main file in multi
+  file projects may be slow. This might happen for instance when there are
+  _very_ many tex files in the directories scanned. In such cases, one may
+  disable the directory scan method by setting this variable to a nonzero
  value.


See earlier comments.

lervag · 2022-11-06T14:41:35Z

doc/vimtex.txt

+2022-11-05: Better main file detection algorithm~
+The detection whether a file is a main file has been improved. In course of
+this *g:vimtex_disable_recursive_main_file_detection* has been deprecated in
+favor of |g:vimtex_disable_directory_scan_main_file_detection|.


If we remove the option, it is still useful to give such a notice.

Yes. An the removal commit would rephrase it accordingly.

shuber2 · 2022-11-06T16:34:27Z

@lervag All good comments, thanks! And I sympathize with "nitpicking". ;)

The method 6 of finding the main file is based on a directory scan which is described as "recursive" scan. However, while a recursive scan suggests a (recursive) directory descent, the opposite is true: the unique ascending path of parent directories is scanned. This is fixed in the following steps: 1. The method is renamed to "directory scan method". 2. The method description is adapted to reflect this behavior more clearly. In particular, it is explicitly mentioned that no recursive directory descent is performed. 3. The option to disable this method is renamed to g:vimtex_disable_directory_scan_main_file_detection 4. Some vimscript comments are fixed that actually mention an upwards recursion in the directory tree. As a side effect, the overall description of the directory scan method is reworked. As a further side effect, the name of this method is more independent of the algorithmic mechanism, but rather reflects the policy, cf. "policy versus mechanism" paradigm

The static function s:findfiles_recursive(expr, path) in state.vim is actually not recursive, but iterates from path upwards in the directory tree. Furthermore, it is an extended version of globpath() with (basically) the same signature. Rename it to s:globpathupwards() to reflect this.

Performance evaluation for the work on PR lervag#2558 gave evidence that disabling the directory scan method for the main file detection due to performance concerns appears to be unjustified. Remove this option. Note that recently g:vimtex_disable_recursive_main_file_detection has been renamed to g:vimtex_disable_directory_scan_main_file_detection.

lervag · 2022-11-06T19:40:08Z

Thanks! I'm glad you don't mind me being pedantic. :)

Looking forward to seeing an updated PR!

shuber2 · 2022-11-06T19:46:44Z

@lervag There we go, let's give it another try. I have split the commits up into logical, independent blocks for their own fate. In particular the commit to remove the disable-method-6 option can be easily reverted by itself.

refer: #2559

lervag · 2022-11-06T20:15:26Z

Thanks! I've merged this now after making a few minor changes. Please feel free to let me know if you disagree with them, specifically this commit: af05c7b.

lervag · 2022-11-06T20:19:06Z

Oh, and by the way: I very much appreciate this contribution. Documentation is important and hard! And this is actually a very important part of the docs. I hope and believe your contribution here may help make this stuff clearer to both new and existing users in the future, and as such, I think it is a very valuable contribution!

shuber2 · 2022-11-06T20:22:24Z

Thank you very much!

lervag reviewed Nov 5, 2022

View reviewed changes

doc/vimtex.txt Outdated Show resolved Hide resolved

doc/vimtex.txt Outdated Show resolved Hide resolved

doc/vimtex.txt Outdated Show resolved Hide resolved

shuber2 force-pushed the doc-directoryscan branch from 5a7a78a to a40c5c9 Compare November 5, 2022 19:47

shuber2 force-pushed the doc-directoryscan branch from a40c5c9 to 90050cc Compare November 6, 2022 07:16

lervag requested changes Nov 6, 2022

View reviewed changes

shuber2 added 3 commits November 6, 2022 19:57

shuber2 force-pushed the doc-directoryscan branch from 90050cc to 80da48d Compare November 6, 2022 19:45

lervag added a commit that referenced this pull request Nov 6, 2022

doc: minor improvements to directory scan descr

af05c7b

refer: #2559

lervag added a commit that referenced this pull request Nov 6, 2022

merge: "recursive search" → "directory scan"

9b191c4

refer: #2559

lervag closed this Nov 6, 2022

doc: rename "recursive search" to "directory scan" #2559

doc: rename "recursive search" to "directory scan" #2559

Conversation

shuber2 commented Nov 4, 2022

shuber2 commented Nov 4, 2022 • edited Loading

lervag commented Nov 5, 2022

shuber2 commented Nov 5, 2022

shuber2 commented Nov 6, 2022

lervag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shuber2 Nov 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shuber2 Nov 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shuber2 Nov 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shuber2 commented Nov 6, 2022

lervag commented Nov 6, 2022

shuber2 commented Nov 6, 2022

lervag commented Nov 6, 2022

lervag commented Nov 6, 2022

shuber2 commented Nov 6, 2022

shuber2 commented Nov 4, 2022 •

edited

Loading

shuber2 Nov 6, 2022 •

edited

Loading

shuber2 Nov 6, 2022 •

edited

Loading

shuber2 Nov 6, 2022 •

edited

Loading