Update from OdatNurd/OverrideAudit@master #2
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
Fix YAML syntax to allow it to be parsed by Python and .NET
Some fixes that were originally made in the version 2.0.0 branch as a part of the BSR have been backported to the legacy branch, where they have been added to the changelog and will be a part of the next release. As such, these changes no longer need to be in the changelog here, since that would look a little weird and wrong.
This is a change that was ported forward from the legacy branch: This completes a change that could have been done a ways back when the final official version of the online documentation was completed, and swaps the command palette and main menu entries for opening the readme file from the package for commands that open the documentation in the user's browser instead. In some future build, this can be swapped for in-Sublime help via hyperhelp.
Back when we were doing BSR to split out the code nicely for version 2.0.0, we fixed a bug in the `save_on_diff` handling wherein if you have the `trim_trailing_white_space_on_save` turned on, a line has any trailing white space AND the file is unsaved, the file will be saved to disk before doing the diff, but it will be left marked as dirty. The fix here is similar to the one created for version 2, but the code is a little more simplistic here.
While making this modification to ensure that overrides that start with a period (e.g. .gitignore) are properly identified, a bug was introduced in which the text in override and diff reports that tells you that there are no overrides was incorrectly identified as an override itself. The syntax test did not catch this due to it checking only for a base scope and not for the exclusion of the override scope as well. In order to resolve this, the syntax has been updated and the text in question is now wrapped in < > marks instead of [ ] since the former is not valid in a filename but the latter is.
This fixes #25 by properly detecting that an unpackaged package is a locally installed dependency. Normal dependencies are installed by PackageControl and have a dependency-metadata.json file created for them in their package directory when they are installed. While developing a new dependency, this file does not exist yet. However in order to have the package properly detected as a dependency PackageControl allows you to create a .sublime-dependency file to indicate the appropriate load order. This commit modifies the behaviour for checking if an unpacked package is a dependency by checking for the presence of that file if the normal one is not there.
This is a simple little change that replaces all instances of the year 2017 in reference to non-changelog type entries to be the most recent year, so that things are more up to date.
This completes a change that could have been done a ways back when the final official version of the online documentation was completed, and swaps the command palette and main menu entries for opening the readme file from the package for commands that open the documentation in the user's browser instead. In some future build, this can be swapped for in-Sublime help via hyperhelp.
This includes a fix for the `save_on_diff` functionality, repairs a regression in the syntax added in version 1.1.0, and replaces the menu and command palette entries that used to display the README file for new ones that open the online documentation in the user's browser. This is an interim point release while we work some more on the external diff functionality that will be in the next version, in preparation for our abandonment of the 1.x versions in favour of the new 2.x BSR version of the code.
https://pyyaml.org/wiki/YAMLColonInFlowContext pull request #28 from forkeith/fix_yaml_syntax
This adds a new setting to the system to specify the name of the external diff program to use on overrides (if any), and how to execute it. This is planned to be done in a way that's extensible in the future and which also supports per-platform settings in the single settings entry so that people that use Sublime on multiple platforms (such as myself) can "set it and forget it" and not have to keep fiddling with settings. whoops
This includes new API endpoints in the PackageInfo class to retrieve the contents of a packed or unpacked override as a list of lines in the file as well as (for packed overrides) an indication as to whether the source is a shipped or installed package. This is a thin wrapper around existing API endpoints that are internal and are used in internal diffing. whoops
This includes a simple helper function whose job it is to take the info on a package and the name of an override, and extract the appropriate packed file out into a temporary file so that it can be examined by tools outside of Sublime. The name of the file created is returned, and consists of information that tells you if the file is shipped or installed, and the full package path and override name (plus an unfortunate 6 character uniqueness suffix on the name proper). The file is created as read only to ensure that the code using the file is made aware that any changes will be lost, since the process is going to remove the file once the diff is completed.
This includes a new thread class for performing an external diff by extracting the base file out of a `sublime-package` file into a temp file (using a recent change) and then executing an external program. This thread silently waits for the process to finish and then attempts to remove the temporary file if it can, although it does not complain if it cannot. This is the core of functionality for opening a diff in an external program.
This does the work of implementing the ability to open a diff in an external program, assuming that one is configured. Currently this is only available from the context menu and command palette from inside of an existing diff window; i.e. there is no direct way to diff externally without opening an internal diff first. This may change either before or after final release, but this is an attempt to keep the clutter in the context menu down by not having the option available in a variety of circumstances since in many cases a more simple diff is probably enough.
In order to ensure that we don't forget to do it in the core repository files, this includes the stub entry in the changelog file for the new external diff functionality, along with the documentation for the new command palette entry and setting.
The previous processing for the external diff setting was to expand all variables across the whole of the argument dictionary, which is not the right thing to do. In particular, Sublime only expands variables in `shell_cmd`, `cmd` and `working_dir`. Additionally, the `env` and `path` keys may also have variables in them, but they are given to the OS to expand out based on their being environment variables. As such, if you try to include `$path` in the `path` key to include the original path, our variable expansion will clobber it out since unknown variables expand as empty strings. The same can happen to `env`. To get around that, we now explicitly only expand variables in the `shell_cmd` and `working_dir` arguments (and in the future, `cmd`, if we decide to support it).
As mentioned in a recent stream, there is a bug in the core exec code regarding the `path` argument in a build system which I was unable to remember how to replicate at the time. The gist of the bug is that if a build system contains a `path` argument the `AsyncProcess` class will save the current path variable and then modify the process environment with the provided path. It will then spin up the child process and reset the path back to what it was. If the call to `Popen` fails (for example with a `cmd` that specifies an invalid program), the exception thrown causes the code to skip over the reset of the environment. Thus in this case the environment is left in a modified state, which affects all future builds. If a person used an invalid or short path (for example to specify just a single folder), the path is forever altered. Although the fix for this is easy enough, the `path` argument is not one of the documented keys in a build system and may be a hold over from a previous Sublime version. Since the same thing can be accomplished with `env`, here we are just redacting the `path` handling code entirely instead.
The code we use is taken originally from the `AsyncProcess` class in the `Default/exec.py` code, and in that class it determines that if there is no working directory variable set, but there is an active file, that it should use that path as the working directory. This is actually a potential problem; if the current file happens to have a file name but not be stored on disk, the working directory will be set to an invalid value and the build (or in our case diff) will fail to execute. Although initial counter-intuitive, this can happen in cases such as if the current file is a package file from a `sublime-package` file that has been loaded by `View Package File` or if a file has been removed from disk, such as during a git operation. Our fix here is to assume that if the user explicitly sets a working dir that they should get the error to let them know that they have done something wrong, but to actually check and not set the working dir to an automatic value if the path is not valid. The idea here is that we should not fail to set an invalid working dir because one that is explicitly set may depend on that working dir and so it is better to fail and let the user fix it than do something untoward.
This introduces the ability to run an external diff program in order to see the results of an override diff in the tool of your choice. This is controlled via a setting by which the user can specify exactly what program they want to execute in order to do the diff and what the command line for that tool should look like. At the present time, this functionality is in the command palette and the context menu of diff views with wording that indicates that you can open this diff in an external tool. A future version might update this to make the original diff open in that tool or something similar.
While I was refactoring and fixing the working directory code I was trying a few different ways to see which I liked better and I accidentally left a change in place.
Just a small tweak, since I recently updated the copyright notice on the license to the latest date, I've decided that I'd like this to be a range of dates instead. That conveys the idea that the code has been around for a while and is thus mature(ish) while also showing that it's still actively maintained.
This generates a dialog box that makes it apparent when an external diff operation has failed, where failure is an indication that the program that was launched returned a non-zero exit code. The most obvious cause of this is a problem with the `shell_cmd` using a program that cannot be found on the path.
This makes some additional changes to the code that determines if we should allow an external diff or not by checking a few common errors. The most important of these is that it used to be possible to specify a value of `true` here, which would generate an error because the code would consider the value a dictionary. Now any boolean value always disables the external diff functionality, and the only thing that enables it is a non-empty dictionary or a string of "sublimerge". Existing logic generates errors if the dictionary does not have the right keys when the diff happens, which saves us the time of having to try and valid the dictionary every time the menu is displayed.
This extends the previous functionality for external diffs by allowing the Sublimerge Pro or Sublimerge 3 package to be used to perform the external diff action by setting the `external_diff` setting to the string "sublimerge". This detects if the package is actually installed and only enables the command when it is believed that the package is ready to go. This includes detection of whether or not the package is currently ignored. There is also a tweak to the detection of external diffing being allowed that ensures that the command is not accidentally visible if the setting of the `external_diff` setting is invalid.
The constituent changes for version 1.1.2 were already in this branch, most having originated from this branch or being backported to the legacy branch. The only change from 1.1.2 that was not present here was previously added in commit cef49e3.
This brings forward the changes from the 1.2.0 code in the stable branch to the new version 2.0 code. The meta data and supporting changes made here are straight forward merges, but the actual code change related to the external diffing code is more involved as the structure of the code changed extensively during the 2.0 BSR.
This brings forward the changes from the 1.2.1 code in the stable branch to the new version 2.0 code. As before, the meta data and supporting changes made here are straight forward merges, but the actual code changes themselves are a bit more involved.
This is a small bugfix release to patch up a defect in the newly added `Open Resource` command. The intent of the newly added `include_existing` argument was to allow for any package resource to be added, but the package filer was still incorrectly filtering away packages that were not packed (since only those packages can contain overrides). This fixes that by applying a different filter that only ensures that disabled packages are filtered away. Thanks to @keith-hall for the bug report.
This factors away some code that used to appear as a class method in the PackageInfo class into a helper function instead. The method in question was used to set up a variable used to store the location for shipped packages. Due to the way that APR reloads packages and the way that the code was structured, hot-reloading the package would cause the shipped package path to be lost. The code has been moved out into a helper method instead, which lazily captures the value on first call.
As previously written, when creating or refreshing any report the cursor would be left at the last line of the report, due to the `append` that is used to populate the report. While this works, it's mildly annoying that on load if you try to move the cursor the view immediately shifts to the bottom of the view, where the cursor is located. This change puts the cursor at 1:1 on create/reload, which more closely mimics how opening a file would act.
As a first addition relating to #35, this adds a new argument to the command that generates override reports that causes it to not display any overrides that diff out to be identical to the underlying file. This is currently exposed as a new entry in the command palette and the `Tools > OverrideAudit` menu to generate a report with the new option, and it also persists in the report so that a refresh will keep the same option. Currently this option does not stop the display of binary files that may or may not be similar (they are currently not diffed or otherwise compared to the underlying file) and it also leaves expired overrides in the list regardless of their content.
Similar to the previous commit, this extends the commands responsible for generating bulk diff reports (single package or multi-package) to include an extra argument that tells the command to elide unchanged files from the report. Any given file will not be included in the diff report if it appears to be unchanged, though unknown and binary files will appear. In the event that the contents of a package contain only unmodified files, that entire package will be dropped from the report entirely. For this reason, the Bulk diff reports now generate a warning at the top to indicate to you in this situation that other files may exist that are unchanged, to remind you that you're not seeing everything there is to see.
This fixes some (mostly hidden) defects in the bulk diffing code that would trigger an error in cases where the package is entirely un-diffable: * When there is only a `sublime-package` file but no unpacked folder * When there is an unpacked folder but no `sublime-package` file Both could cause the diff code to throw an exception instead of gracefully handling the issue. This is mostly hidden because the outer command is normally used in a way where it prompts you for the package(s) to diff, and in such a case it would not allow you to choose a package that would cause this problem to surface. As such, the bug fix is to allow for others to invoke the command and get a meaningful result rather than an exception without their having to know ahead of time if the package is safe or not (which is a non-obvious check for outside use cases).
Our entries in the tab context menu could cause errors to be thrown in the console (and the command to be unavailable in the menu) if the tab prior to a report or an override was a non-`TextSheet`, such as an image. This is because our command gets told what sheet index it is in the group, but we were using `window.views_in_group()` to look up the group. We now use `window.sheets_in_group()` instead, so that the index we're given is always valid.
In report views, we now apply our own custom context menu that only shows relevant commands, which are OverrideAudit commands only. In order to make this more seamless, commands have been modified so that their description methods know if they should include the "OverrideAudit: " prefix or not; they will not when used in a report but they will in other cases.
While reviewing in preparation for a possible release, I realized that although I set up the entry and stub file for the next version, I didn't actually update it.
This release brings some bug fixes and includes the ability for override and diff reports to optionally exclude overrides that are unchanged, showing only modified, unknown and binary resources.
Adjust the message in the release instructions to better reflect how our tagging works and is done, so that I don't have to keep double checking every time.
The python version displayed for the User package should be 3.3 if you're using Sublime Text 3, and 3.8 if you're using a Sublime Text 4 build. This makes sure that the version is always correct based on your running version.
In ST3 the only time the `event` argument gets provided to a command is when it's invoked from the context menu inside of a view (and it actually requested to get the event). In ST4, the `event` argument will also be raised on commands in the tab context menu to provide keyboard modifier key information. The context commands were running afoul of this in Sublime Text 4 because they ask for an event but expect it to always contain position information. This simple fix only assumes that an event should be handled if it exists and also contains positional information.
This release fixes a small visual bug in the display of the version of Python used to run plugins in the User package, and fixes tab context menu items in Sublime Text 4, which were throwing errors due to changes in the handling of the event argument.
In the command palette, OverrideAudit: Open Resource executes command: override_audit_create_override {"include_existing": true}
Prior to this commit, it was always firing the PackageResourceBrowser with "unknown" set to `False`.
But, as the command palette entry's name indicates, it would be expected to be able to select *any existing* resource from this list - whether it only exists as an extra file in the package or not. (This is important when creating new syntax definitions which extend other ones, for example - they won't exist in the original package, but we still want to see them and open them.)
The solution therefore is to set `unknown` to the value of `include_existing`, to keep it working the same way for other use cases,
while fixing the aforementioned command palette entry.
Fix Open Resource command palette entry not showing all files
This fixes a small defect in the Open Resource command palette item, which has an argument that indicates whether or not unknown overrides (files which appear in an unpacked package but not in the sublime-package file) should appear in the list. The argument was not being used and always used a value of False, meaning that when browsing for an override to open, unknown overrides were not open-able.
Previous to this, the code in the PackageInfo class would assume that the User package runs in the 3.8 host if it's a ST4 build, and all other packages run in either the legacy 3.3 host or, if they have .plugin-version file, in that host. The Default package is special in that it "straddles" the plugin host and is available in both of them in a way that other plugins aren't. This[ this alters the code so that we can say that the Default package will run in multiple hosts where available in the package popup, which is more accurate.
In this context, a "package specifier" is the text `[SIU]` that appears in override and diff reports, and the `[S]`, `[I]` and `[U]` values in the package report that tell you what kind of package it is. All syntaxes now apply meta scopes to these things, and the tests have been augmented as well to verify them. This will allow us to create more hover popups.
Include the ability to provide hover popups that provide more information on specific topics, as outlined by the text being hovered over. This is implemented for the package type specifiers in reports (including the specific ones in a Package report) as well as for the markers for the different types of overrides (expired, unknown, complete, expired complete). When hovering over some items, links are available in the popup that will execute commands, including creating a diff report or a new override, both in the package that the item hovered over is associated with.
This release brings a couple of minor changes, most notably in the hover popups that we provide on reports. The Default package is now more visibly documented to be available in the 3.3 and 3.8 hosts, and there is now hover help for some of the report specifiers, such as [X] and [S] and the shorter form [SIU] varieties. This build is the last build that is going to be made available for Sublime Text 3 users.
There are no functional changes in this release; the only change here is that the URL for the documentation has been updated in a few places because the domain and hosting that is being used for the documentation has been updated. As such, this release is also being pushed for ST3 users, just so that people using older versions can still reach the documentation site without getting a 404 error.
This is a small point release to fix a documentation URL that was missed in version 2.2.6. As such, this is also going out to Sublime Text 3 users.
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.
The upstream repository
OdatNurd/OverrideAudit@masterhas some new changes that aren't in this fork. So, here they are, ready to be merged!This Pull Request was created programmatically by the githubpullrequests.