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

DOC: Adjust title of ImageBufferAndIndexRange example #386

Conversation

N-Dekker
Copy link
Collaborator

Following the guideline on how the write a title, at
https://examples.itk.org/documentation/contribute/writeanewexample#title
which suggests filling out the dots ("...") of the question, "How do I ...?"

Following the guideline on how the write a title, at
https://examples.itk.org/documentation/contribute/writeanewexample#title
which suggests filling out the dots ("...") of the question, "How do I ...?"
@github-actions github-actions bot added area:Core Issues affecting the Core module area:Documentation Issues affecting the Documentation module labels May 23, 2022
@N-Dekker
Copy link
Collaborator Author

@thewtex @tbirdso Is it OK to you to extend the title to "Iterate Over an Image Buffer and an Index Range", while still keeping the name of the directory and the test project short (ImageBufferAndIndexRange)?

I'm asking specifically because on Windows, long directory path names still appear troublesome, nowadays! So I'm reluctant to rename the directory to something like "IterateOverAnImageBufferAndAnIndexRange". Unless you insist 😃

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Are there other examples of larger discrepancy between directory name and example title?

@tbirdso
Copy link
Contributor

tbirdso commented May 23, 2022

@N-Dekker I agree with the motivation to keep the directory name short if possible. I am not sure if the docs style check will allow an example name that differs from the parent directory. Let's see whether build-test-documentation returns a warning.

@N-Dekker
Copy link
Collaborator Author

FYI, I just figured out (*) that the longest ITKSphinxExamples src directory path name so far is:

Filtering\ImageFilterBase\PredefinedOperationToCorrespondingPixelsInTwoImages

So Core\Common\IterateOverAnImageBufferAndAnIndexRange would still be shorter than the longest existing path anyway.


(*) PS I estimated the longest directory path name by the following code: https://godbolt.org/z/hdKneP97P

#include <filesystem>
#include <iostream>

int main()
{
  size_t longestPathLength{};

  for (const auto& entry : std::filesystem::recursive_directory_iterator("."))
  {
    const auto path = entry.path();
    const auto pathLength = path.string().size();

    if (std::filesystem::is_directory(path) && pathLength > longestPathLength)
    {
      longestPathLength = pathLength;
      std::cout << longestPathLength << " chars: " << path.string() << std::endl;
    }
  }
}

@N-Dekker
Copy link
Collaborator Author

When I tried to build ITKSphinxExamples for the first time (on Windows), I did encounter those CMake errors, even while my local source and build directory path names were rather short, in my opinion (something like "D:\X\source\ITKSphinxExamples" and "D:\X\build\ITKSphinxExamplesSuper"):

if(${source_len} GREATER 30)
message(FATAL_ERROR
"The source directory is currently too long, ${source_len} characters. "
"Please move the Examples source directory to a shorter path."
)
endif()
string(LENGTH "${ITKSphinxExamples_BINARY_DIR}" binary_len)
if(${binary_len} GREATER 30)
message(FATAL_ERROR
"The build directory is currently too long, ${binary_len} characters. "
"Please set the Examples build directory to a shorter path."
)

@N-Dekker N-Dekker marked this pull request as ready for review May 23, 2022 15:38
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Yes, we may need to diverge on the Windows and example name for path length issues.

@tbirdso tbirdso merged commit 18a99c7 into InsightSoftwareConsortium:master May 23, 2022
@tbirdso
Copy link
Contributor

tbirdso commented May 23, 2022

@N-Dekker Checks passed, great! Yes, Windows paths are a pain point here, it may be worthwhile for us to revisit and rename example directories with long titles as you found. In CI we use C:/Ex for the source directory and C:/Ex/ITK-build for the build directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module area:Documentation Issues affecting the Documentation module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants