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

Hexyll.Core.Identifier.Pattern をリファクタリングする #67

Closed
Hexirp opened this issue Dec 30, 2019 · 22 comments · Fixed by #68
Closed

Hexyll.Core.Identifier.Pattern をリファクタリングする #67

Hexirp opened this issue Dec 30, 2019 · 22 comments · Fixed by #68

Comments

@Hexirp
Copy link
Owner

Hexirp commented Dec 30, 2019

きちんとした Monoid になっていない、そもそも DSL の形が気に入らない、ベースに regex-tdfa を採用している、など気に入らない所がいっぱいある。

言語拡張も Internal モジュールを統合したので必要なくなった。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

fromList のドキュメントに注意書きがあるけど、そもそも version を Pattern に混ぜんなよという感がする。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

躊躇なく新しい設計を組み立てようとする。具体的には PatternIdentifier -> Bool の newtype にする。ここで問題になるのは Pattern が Binary である必要があるのかどうか。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

jaspervdj/hakyll@d2aaf33jaspervdj/hakyll@86ede74 が関わっているみたい?

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

Dependencies を扱うために必要なそうで。具体的には https://github.com/Hexirp/hexirp-hakyll/blob/a1dd75a18c5b3a55c2684326d93896c4c1e1d672/hexyll-core/src/Hexyll/Core/Dependencies.hs の所。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

--------------------------------------------------------------------------------
type DependencyM a = RWS [Identifier] [String] DependencyState a

あっ。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

対策するためには、別々の型にするとかかな?

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

regex-applicative を使うのはやめておこう。 Eq でも Show でもなくなるから。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 30, 2019

あー、でも、直接 Pattern に変換すればいいのか?

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

まあ、ここがあかんのだよね。

mOldFacts <- Store.get store factsKey

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

ここもあかん。

Store.set store factsKey $ runtimeFacts s

その原因は

, runtimeFacts :: DependencyFacts

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

なんで普通に列挙しないでパターンも許しているの?

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

OldPattern の中で代替できなさそうなのは capture 関連か。ここの意味がよくわからない。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

「型を合わせる」とは coerce で済む作業である。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

capture は使われていないので消す。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

capture はこんな感じのようだ。つまりワイルドカードにマッチした箇所をリストにして返す。マッチしなかったら Nothing になる。

--------------------------------------------------------------------------------
captureTests :: [TestTree]
captureTests = fromAssertions "capture"
[ Just ["bar"] @=? capture "foo/**" "foo/bar"
, Just ["foo/bar"] @=? capture "**" "foo/bar"
, Nothing @=? capture "*" "foo/bar"
, Just [] @=? capture "foo" "foo"
, Just ["foo"] @=? capture "*/bar" "foo/bar"
, Just ["foo/bar"] @=? capture "**/qux" "foo/bar/qux"
, Just ["foo/bar", "qux"] @=? capture "**/*" "foo/bar/qux"
, Just ["foo", "bar/qux"] @=? capture "*/**" "foo/bar/qux"
, Just ["foo"] @=? capture "*.html" "foo.html"
, Nothing @=? capture "*.html" "foo/bar.html"
, Just ["foo/bar"] @=? capture "**.html" "foo/bar.html"
, Just ["foo/bar", "wut"] @=? capture "**/qux/*" "foo/bar/qux/wut"
, Just ["lol", "fun/large"] @=? capture "*cat/**.jpg" "lolcat/fun/large.jpg"
, Just [] @=? capture "\\*.jpg" "*.jpg"
, Nothing @=? capture "\\*.jpg" "foo.jpg"
, Just ["xyz","42"] @=? capture (fromRegex "cat-([a-z]+)/foo([0-9]+).jpg") "cat-xyz/foo42.jpg"
]

そして fromCapture は * とか ** の穴を与えられた値で埋めていく。

-- Example:
--
-- > fromCapture (fromGlob "tags/*") "foo"
--
-- Result:
--
-- > "tags/foo"

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

てゆーか、これ regex-applicative を使えば行ける話じゃん。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

PatternData と Pattern の Monoid 構造は compile により準同型になっている。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

ドキュメントを追加したけど、なんか見づらいような気がする。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

こことか

--------------------------------------------------------------------------------
match :: Pattern -> Rules () -> Rules ()
match pattern = matchInternal pattern $ getMatches pattern

ここで

--------------------------------------------------------------------------------
-- | Apply the route if the identifier matches the given pattern, fail
-- otherwise
matchRoute :: Pattern -> Routes -> Routes
matchRoute pattern (Routes route) = Routes $ \p id' ->
if matches pattern id' then route p id' else return (Nothing, False)

古い Pattern が使われている。

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

うーん、 (.||.) とかは Regex とかでできるから捨てちゃう?

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

そもそも現実的には Glob しか使わない?

@Hexirp
Copy link
Owner Author

Hexirp commented Dec 31, 2019

なぜ Old.PatternBinary のインスタンスを持っていなければならないのか?

Hexirp added a commit that referenced this issue Dec 31, 2019
Fix #67 .

Hexyll.Core.Identifier.Pattern をリファクタリングする。実際には
Pattern モジュールを OldPattern モジュールにリネームして Pattern
モジュールに新しい実装を作成するという形となった。

まだ不完全であるが、これ以上は Dependencies モジュールやその他の
上流モジュールでの使われ方を見てから作業したいので、ここで終わりに
することにした。
Hexirp added a commit that referenced this issue Jan 4, 2020
See #67 .
See #68 .
See #69 .

再び `Hexyll.Core.Identifier.Pattern` をリファクタリングする。
前にも同じような作業を行ったが、そのプルリクエストに書いた通り、
それはたたき台としての実装でありまだ不完全であった。そのため、
`Hexyl.Core.Dependencies` など他のモジュールでの使われ方を見て
から、さらにリファクタリングを行う予定であった。

実際にそうした結果、たたき台としてのものは実際の用途に合っていない
ことが分かったため再びリファクタリングすることになった。

まず、たたき台としての実装は `Monoid` のインスタンスが本当の
モノイドになっていないという問題を解決しようと試みるものであった。
そのために `(.||.)` と `complement` を排除した代わりに `Binary` の
インスタンスを持つ `PatternData` と、自由に `(.||.)` や
`complement` を使える代わりに `Binary` のインスタンスを持つ
`Pattern` に分離した。

しかし、 `Binary` のインスタンスを持つということは重要な性質である
ことが分かったため、論理的な操作を自由に使えるが `Monoid` の
インスタンスを持たない `PatternExpr` を定義することにした。
そして、それをリストで包んでモノイドにした `PatternConj` と
`PatternDisj` を定義した。この両者は `match****` 関数において
モノイドの作用が `(.&&.)` と `(.||.)` のどちらかとして扱われるかに
よって区別される。また、真なる自由なパターンを表すものとして
`Pattern` を残した。

`Hexyll.Core.Dependencies` など普通の用途としては `PatternExpr`
を使い、 `Hexyll.Core.Rules.Internal` など `Monoid` のインスタンスが
要求される個所では `PatternConj` か `PatternDisj` を使うことを
想定している。

この変更により、実際の用途まで踏み込んでいなかったドキュメントにも
実際の用途についての有意義な情報を書けた。

テストはまだ記述していないが、それは後で実装する。また、 `NFData` の
インスタンスも作成したい。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant