Separate prefix options for directories and filenames in partial references #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds to test/fixtures/partials/sub-a and test/fixtures/partials/sub-b for future unit tests involving testing partial references that use directories rather than just bare filenames. Included in each subdirectory are three partials analogous to those found in the parent directory.
Introduces a new test to the testing suite that combines the existing prefix option with a partial reference that uses a subdirectory. The aim of this test of to ensure that the (arguably weird) behavior of applying a prefix to the directory works in future versions of the plug-in after changes are made to the prefix handling.
Introduces another unit test for the purposes of making sure that a user who specifies a partial relative to the base of their project (i.e. such a reference where they have left the 'directory' option blank) gets the correct result with the upcoming prefix changes.
Adds support for these new options, including mapping usage of the old-style prefix option to the new option depending on what kind of partial reference is encountered. For bare filenames, prefix behaves like filenamePrefix. For paths including a directory, prefix behaves like directoryPrefix (which matches the current weird behavior).
Adds unit tests for filenamePrefix, directoryPrefix, and a combination of the two. Reuses existing fixture data and expectation output, except for the combination unit test which uses new expectation output.
If the user attempts to use a partial reference that does not exist or tries to use the old-style prefix option with a path, prints a warning to the terminal.
Prints a warning to the terminal if the plug-in encounters a bare filename partial in a ruleset that has specified a directoryPrefix.
Adds directoryPrefix and filenamePrefix to the options list, replacing the old prefix option.
Switches directoryPrefix to prefix_dir and filenamePrefix to prefix_file, which more closely matches the existing naming convention used by the other options. By renaming directoryPrefix, this also avoids the inherent confusion with the unrelated 'directory' option.
I did not end up using sub-b/ and also do not test alternate extensions in combination, these files can safely be removed.
|
Just came back from vacation and have to work on something else atm. Would you please ping me if I forget to get back on this issue? Thanks in advance. |
|
Of course I forgot about it :) Thanks for the changes. This really makes it ultra flexible from what I can see in the unit tests. Great changes, will test them (hopefully) soon. |
|
No problem. It's not too late for other changes if you find anything that needs fixing. Thanks, @franz-josef-kaiser! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm introducing this as a possible resolution for issue #9.
This changeset adds options support for
prefix_dirandprefix_file, which clarifies prefix configuration for partial references that use a path. The originalprefixoption continues to be available, and has the same behavior as before. The included documentation recommendsprefix_dirandprefix_filebut still mentionsprefix.Also included are new unit tests for a partial reference using a path but no configured partial directory or prefix (which works in the current version),
prefixusing a path (to test the old weird behavior),prefix_fileby itself,prefix_dirby itself, andprefix_file/prefix_dircombined.@5thWall, @franz-josef-kaiser: Feel free to take a look and let me know if you see any issues or problems with this approach.