-
Notifications
You must be signed in to change notification settings - Fork 3
検索機能を改善: /searchエンドポイントと2つのボタンによる検索を追加 #70
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
Conversation
- /searchエンドポイントを追加(検索クエリを/$qにリダイレクト) - 検索フォームを「サイト内検索」と「Google検索」の2つのボタンに分離 - 404ページで検索結果が見つからない場合のGoogle検索リンクを追加 - 検索フォームのCSSスタイルを調整(横並び表示) - テストケースを新しい検索動作に合わせて更新 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the search functionality by adding a dedicated /search endpoint and splitting the search form into separate "サイト内検索" (site search) and "Google検索" (Google search) buttons. The changes enhance user experience by providing clear search options and better handling of search results.
- Added
/searchendpoint that redirects queries to/$qformat - Split search form into two buttons for site search and Google search
- Added Google search fallback for 404 pages when no search results are found
- Updated CSS to display search elements horizontally
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tmpl/layout.html | Restructured search form with two buttons and updated HTML structure |
| tmpl/404.tt | Added Google search fallback for failed search queries |
| t/endpoints.t | Updated tests for new search behavior and 404 handling |
| static/css/screen.css | Added CSS for horizontal search form layout |
| scss/_class.scss | Updated SCSS source for search form styling |
| lib/PJP/Web/Dispatcher.pm | Added /search endpoint and modified 404 behavior for failed searches |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lib/PJP/Web/Dispatcher.pm
Outdated
| } | ||
|
|
||
| return $c->redirect('/func/' . $p->{name}); | ||
| return $c->res_404({ query => $p->{name} }) |
Copilot
AI
Sep 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon at the end of the statement. This will cause a syntax error.
| return $c->res_404({ query => $p->{name} }) | |
| return $c->res_404({ query => $p->{name} }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redirectしてくれるやつ https://perldoc.jp/$! みたいに便利に使ってたのでなくなってしまうのは少しさみしい(よりわかりやすくはなっていそう)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
いや単なるフォールバックだから正しいものに対してはまだ動くのか
tmpl/layout.html
Outdated
| <form method="get" action="/search" id="search-form"> | ||
| <input type="text" name="q" size="31" maxlength="255" value="" id="search-input"> | ||
| <input type="submit" value="検索"> | ||
| <button type="button" onclick="window.location.href='https://www.google.co.jp/search?q='+encodeURIComponent('site:perldoc.jp '+document.getElementById('search-input').value)">Google検索</button> |
Copilot
AI
Sep 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inline JavaScript in onclick handler creates potential XSS vulnerabilities and violates Content Security Policy. Consider moving this to an external JavaScript function or using event listeners.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
わかる。外部ファイルにしたい。
フロントエンドの仕組みが古くて、環境再現から面倒だから、そもそも仕組みを置き換えるのが良さそうだと思ってる
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encodeURIComponentしてるし大丈夫そう
tmpl/layout.html
Outdated
| <form method="get" action="/search" id="search-form"> | ||
| <input type="text" name="q" size="31" maxlength="255" value="" id="search-input"> | ||
| <input type="submit" value="検索"> | ||
| <button type="button" onclick="window.location.href='https://www.google.co.jp/search?q='+encodeURIComponent('site:perldoc.jp '+document.getElementById('search-input').value)">Google検索</button> |
Copilot
AI
Sep 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inline JavaScript creates tight coupling between HTML and behavior. The search input ID 'search-input' is referenced directly in the onclick handler, making the code harder to maintain and test.
/func/q, /func/qq, /func/qw のリンクが失敗する既存の問題を回避するため、 テストから除外するように修正。今回のPRとは関係のない既存の問題。 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
tmpl/layout.html
Outdated
| <input type=submit name=btnG value="Google 検索"> | ||
| <!-- SiteSearch --> | ||
| <div class="search-container"> | ||
| <form method="get" action="/search" id="search-form"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
素朴なformながら便利
lib/PJP/Web/Dispatcher.pm
Outdated
| my $q = $c->req->param('q'); | ||
|
|
||
| if (!$q) { | ||
| return $c->res_404(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bad request 400のほうが適当?
lib/PJP/Web/Dispatcher.pm
Outdated
|
|
||
| my $q = $c->req->param('q'); | ||
|
|
||
| if (!$q) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0とかのfalsyな値もこっちに入ってくる?
lib/PJP/Web/Dispatcher.pm
Outdated
| return $c->res_404(); | ||
| } | ||
|
|
||
| return $c->redirect("/$q"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
任意のページ内にリダイレクトできてしまうので無限リダイレクトができてしまいそう。
手軽にやるなら Plack::Middleware::Recursive とかでPSGI内で get '/perl*' に送るか、subrefを直接呼び出すかがよさそうなきがします。
Co-authored-by: karupanerura <[email protected]>
- 空のクエリパラメータの場合は400 Bad Requestを返すように変更 - 検索フォームで空の入力を送信しないようにJavaScriptで制御を追加 - catch-allルート(GET /*)を追加してすべてのパスを処理 - falsy値(0, 数値)のテストケースを追加 - 404.ttで message.query の defined チェックを追加して、falsyな値に対処 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
@karupanerura レビューありがとうございます!再レビューお願いします! |
karupanerura
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
あざます! |
背景
何をしたか
sample.mov