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

Add wildcard passthrough #31

Merged
merged 5 commits into from
Aug 24, 2019
Merged

Conversation

C0urante
Copy link
Contributor

Most URL patterns behave nicely with Zap as-is, but there are a few exceptions. One gap I've encountered is with URLS that embed version numbers as path elements; for example, the Apache Kafka docs use the major and minor version number concatenated (with no dot delimiter) as their first path element and each version of their docs so far has followed the same pattern for where javadocs, quickstarts, configuration docs, etc. are found.
I've had to install several redundant redirects in my Zap config to work with this:

ak:
  expand: kafka.apache.org
  "10":
    expand: "10"
    j:
      expand: javadoc/index.html?overview-summary.html
    d:
      expand: documentation.html
  "11":
    expand: "11"
    j:
      expand: javadoc/index.html?overview-summary.html
    d:
      expand: documentation.html
  # ...more of the same...
  "23":
    expand: "23"
    j:
      expand: javadoc/index.html?overview-summary.html
    d:
      expand: documentation.html

However, that's clearly a workaround and it'd be nice if Zap recognized URL patterns like this natively.
As a light first pass at addressing this issue, I'd like to propose that the special path of "*" be allowed to match any path element that isn't explicitly matched in the config file. Should this match occur, the path element will then be kept as-is, but also allow for expansion of the remaining path elements.
An example is included in the README, and a few tests are added that should also help demonstrate the intended functionality. To put things into context, the above config snippet could now be reduced to:

ak:
  expand: kafka.apache.org
  "*":
    j:
      expand: javadoc/index.html?overview-summary.html
    d:
      expand: documentation.html

@C0urante
Copy link
Contributor Author

@issmirnov enjoying this project so far, thought this change might be useful. Looking forward to hearing what you think!

Copy link
Owner

@issmirnov issmirnov left a comment

Choose a reason for hiding this comment

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

Thank you, looks like a fantastic addition! Left a few nits in the code.

config.go Outdated Show resolved Hide resolved
text_test.go Show resolved Hide resolved
@issmirnov
Copy link
Owner

@C0urante Thank you, really excited to get this PR. I've fixed travis to build PR's now, so we should get some more data after the next push.

text.go Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 24, 2019

Codecov Report

Merging #31 into master will increase coverage by 2.4%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #31     +/-   ##
=========================================
+ Coverage   49.77%   52.17%   +2.4%     
=========================================
  Files           4        4             
  Lines         219      230     +11     
=========================================
+ Hits          109      120     +11     
  Misses        103      103             
  Partials        7        7
Impacted Files Coverage Δ
config.go 32.92% <ø> (ø) ⬆️
text.go 83.33% <100%> (+3.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7aaf69d...057a69f. Read the comment docs.

@issmirnov issmirnov merged commit 2af90df into issmirnov:master Aug 24, 2019
@issmirnov
Copy link
Owner

@C0urante Fantastic work, thank you very much for a great addition!

issmirnov pushed a commit that referenced this pull request Aug 24, 2019
- Add wildcard passthrough
- Ignore reserved keywords when present in URL paths
@issmirnov
Copy link
Owner

Had to fight a few bugs with goreleaser, but ultimately got the release up. Homebrew cask has been updated as well at issmirnov/homebrew-apps@3b33e0b

@C0urante C0urante deleted the wildcard-passthrough branch April 15, 2021 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants