Allowing the git_hook function to accept a user-specified settings_path.

[diseraluca]

The current git pre-commit hook function seems to be taking a list of staged files and using the directory containing the first of those files, when at least one file is present, as the base path from which the configuration file is searched for, by going upward into the directory structure until a config file is found or MAX_CONFIG_SEARCH_DEPTH is exhausted.

This means that for the config file to be found it has to be in a directory
between the first staged file and its at most MAX_CONFIG_SEACH_DEPTH parents.

While this might generally be true, I feel that we cannot expect that this is
the case. For example, we might imagine the following structure:

git-root/
  config/
    .isort.cfg
  src/
    ...

If any file in src is modified, which will probably be the case when that file
is a .py file, the config file will not be found.

If this structure should not be supported such that a specific project structure
is expected to be in place, then I feel that a better heuristic would be to
simply look at the git repository root directory ( that should be found able
with git rev-parse) as that is where the config file will usually live and
allows us to avoid some possible error in supporting structures that are not
expected to work . Implementing this, tough, breaks any project that might be
using a per directory config file, for example:

git-root/
  src/
    path1/
      .isort.cfg
      ...
    path2/
      .isort.cfg

While I feel that this is improbable, I don't feel that such a backward-breaking
change might be acceptable. Nonetheless, this is not completely supported anyway
as only the first file path will be checked.

If we want the change to be less breaking, we could keep the current
heuristic but look from the topmost directory and go downward, with the
possibility of restricting this to at most the root of the current git
repository. But even this, while I feel it is a better heuristic, does not deal
with the first case. This would further break the current behavior if two
config files are on the same path, as we will reverse the order in which they
are found.

A different approach might be to start at some point, for example the root of
the git repository, and do a breadth-first-search into the repository, but that
would then, again, break the current behavior as the search stops at the first
file, which is now found in reverse order. Further, it might be particularly
slow in some cases where many branches exist in the directory structure and the
config file is at some high depth.

This too is solvable but, in general, any of those changes is incomplete in
allowing one or the other structure considering the current behavior.

Instead, I feel that it might be sensible to allow for the hook function to
accept either a settings file outright or to accept a path that should be used
as the base path for the search. We should be able to implement this without
changing the current behavior while allowing us to accept project structures
such as the one above.

Do you think this might be something that you would allow the project to
implement?

Now, I may be completely misunderstanding the code, as this is the first time
that I'm using isort and looking at its code. If that is the case, I apologize
both for posting wrong information and for wasting your time. Further, I may be
missing a solution that is already sensible with the current implementation. In
that case I would thus apologize, again, and would be thankful if that will be
taught to me.

If I'm not mistaken and this is a desired change, I'm available to post a PR
myself if you so desire.

As a side not, thank you for working on this project and for making it
available; I feel that it is incredibly useful and well made, however that is
worth.

[timothycrosley]

I think your assessment is spot-on.

Instead, I feel that it might be sensible to allow for the hook function to
accept either a settings file outright or to accept a path that should be used
as the base path for the search. We should be able to implement this without
changing the current behavior while allowing us to accept project structures
such as the one above.

That sounds like a great solution to me!

If I'm not mistaken and this is a desired change, I'm available to post a PR
myself if you so desire.

A PR that implemented this would be very appreciated and quickly accepted.

As a side not, thank you for working on this project and for making it
available; I feel that it is incredibly useful and well made, however that is
worth.

Thank you, I really do appreciate that! There are still many ways for it to be improved - but am happy to work alongside others in the community to continually improve it :).

Thanks!

~Timothy

[diseraluca]

A PR for this issue was pushed at #1400.

[timothycrosley]

Thanks for implementing this! You're feature has been merged into develop and will be deployed to PyPI with the 5.5.0 release of isort.

Thanks!

~Timothy

[timothycrosley]

This change has just been deployed to PyPI in version 5.5.0

Thanks!

~Timothy