Skip to content
This repository has been archived by the owner on Dec 12, 2022. It is now read-only.

コード整形 #40

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

コード整形 #40

wants to merge 27 commits into from

Conversation

ethertank
Copy link
Contributor

コードの書式にブレがあり、更新の妨げとなる為、これらを一括して統一しています。

  • HTML の空要素 ( input / br ) 末尾のスラッシュ前のスペースを「有り」に統一。
  • CSS ブロック最終行末尾のセミコロンを「有り」に統一。
  • PHP の関数ブロック前のスペースを「有り」に統一。
  • PHP の else / if else などの前後の改行を「無し」に統一。前後のスペースを「有り」に統一。

また、一部視認性向上を目的とした好みによる改行の 追加/ 削除 も行っています。

saneyuki_s and others added 3 commits April 7, 2013 00:50
コード書式にブレがあり、更新の妨げとなる為これらを一方に統一。単一文の php ブロック内部の改行を削除するなど、好みによる変更も行っている。

HTML の空要素の末尾スペースを「有り」に統一。
CSS ブロック最終行 へのセミコロンを「有り」に。
PHP の関数ブロック前のスペースを「有り」に。
単一行で良いと思われる複数行コメントを単一行に変更。
…など。
@tetsuharuohzeki
Copy link

ethertank@41f8e9c にて、結構な数のファイルが全面書き換えになっていますが、なぜですか? (例: author.php )

  • HTML の空要素 ( input / br ) 末尾のスラッシュ前のスペースを「有り」に統一。
  • CSS ブロック最終行末尾のセミコロンを「有り」に統一。
  • PHP の関数ブロック前のスペースを「有り」に統一。
  • PHP の else / if else などの前後の改行を「無し」に統一。前後のスペースを「有り」に統一。

のルールではこのような大規模変更は考え難いのですが;

@ethertank
Copy link
Contributor Author

概ね申し上げた通りの変更しか行っていない筈です。

@tetsuharuohzeki
Copy link

  • allfeeds.php
  • allposts.php
  • archive.php
  • author.php
  • developer.php
  • header.php
  • index.php
  • project-list.php
  • rtl.css
  • search.php
  • sidebar.php
  • single-contact_page.php
  • single-event.php
  • single-project.php
  • single.php
  • style.css

の変更は、改行コードが意図せず変更されてしまっているのが原因です。git amendなどで改行コードを元に戻してください。

改行コード
rtl.css, style.css (CR LF)
rtl.css, style.css (CR)
rtl.css, style.css (LF)
rtl.css, style.css (CR LF)
@ethertank
Copy link
Contributor Author

CSS ファイルがどの改行コードを使用しているか分からず、 "CR" 、 "LF" 、 "CR LF" を試しましたが、どの場合でも「全面書き換え」のように表示されました。結局、 mozilla-japan / modest_themes の master のファイルをエディタで確認し、"CR LF" のようだったのでこれに戻しています。

@tetsuharuohzeki
Copy link

結局、 mozilla-japan / modest_themes の master のファイルをエディタで確認し、"CR LF" のようだったのでこれに戻しています。

私の手元で mozilla-japan / modest_themes:masterのファイルを確認した所、改行コードはLF(UNIX)のものが多数。一部ファイルのみがCR LF(Windows)という状況でした。

食い違いがあるようなので、http://www.kashim.com/kanjitranslator/index.html などを使って、確認してもらえますか?

私の確認した所、CR+LFだったものは

  • admin.css
  • 404.php
  • page.php
  • rtl.css
  • style.css

です。また、同時に調べてみましたが、改行コードに限らず、さらに一部の文字エンコードがutf-8ではないのも確認しました。
この件については、別途issueを立て、改行コードおよびファイルの文字エンコードを統一します。

@ethertank
Copy link
Contributor Author

CSS ファイルがどの改行コードを使用しているか分からず、

やや分かりにくかったかもしれませんが、 CR LF だったと私が言っているのは style.css と rtl.css についてです。
おっしゃる通り、CSS 以外のファイルも混在しているようですね。

ご提示いただいたソフトは確認用ではなく書き換え用のソフトの様ですが、これを使って変換後、コミットして確認せよとの事でしょうか?

@tetsuharuohzeki
Copy link

ご提示いただいたソフトは確認用ではなく書き換え用のソフトの様ですが、これを使って変換後、コミットして確認せよとの事でしょうか?

いえ、統一すべきファイルがこれで大丈夫なのかについて、現状の確認だけお願いします。
改行コードと文字コードの変更については、私の方で #42 で一旦Pull Requestを出しました <(_ _)>

@ethertank
Copy link
Contributor Author

おっしゃる意味が理解できなかったので、ご紹介いただいたソフトは用いずに改行コードについて確認しました。
master の、以下のファイルが改行コードに CR LF を用いているようです。

  • admin/admin.css
  • js/admin-script.js
  • js/function.js
  • 404.php
  • page.php
  • rtl.css
  • style.css

@tetsuharuohzeki
Copy link

返信遅れてすみません。

おっしゃる意味が理解できなかったので

一部のエディタで、英数字しか使っていないBOM無しのutf-8文字コードのファイルの文字エンコードや改行コードを誤認するバグなどが存在しているため、確実性を重視した為、そのソフトを紹介しました。言葉足らずですみませんでした。

さて、長丁場に成ってしまった本件の解決について、私が現在考えているフローとしては、

  1. @ethertank さんが一番最初にプルリクエストしたコードの書式整形に関するものだけをマージ
  2. 1.の後、文字コード・改行コードの統一について、それぞれ一つのコミットに集約したものをマージ

の順番で、この issue を無事 close できるのではないかと思っています。

@ethertank さんは、どう思われますか?

@ethertank
Copy link
Contributor Author

先に @saneyuki さんに #42 で統一していただいて先にマージ。こちらも全て統一から再コミット。問題が無ければマージ…という形になるのかな…と考えていました。そうすれば、このコミットの差分も確認しやすいのではないかと…。

お任せします。

@tetsuharuohzeki
Copy link

わかりました。先に #42 をマージします。

@tetsuharuohzeki
Copy link

CLOSED: #42

それでは、お願いします。

@ethertank
Copy link
Contributor Author

全て統一したつもりですが… page.php 、404.php 、 それと css の 3 ファイルの差分がおかしいですね…。

@ethertank
Copy link
Contributor Author

お手数ですが、現在の master に Shift-JIS のものが混入していないかご確認いただけないでしょうか?
こちらで確認する限りでは、 admin/admin.css などの幾つかのファイルが Shift-JIS になっているような気がします。

私の間違いでしたら申し訳ないですが、ご協力宜しくお願いします。

@tetsuharuohzeki
Copy link

nkfコマンドで確認した所、確かに一部のファイルがascii扱いになってますね……

utf-8で保存していますが、ファイル内容がasciiコードのみで出力されている為の誤検知かもしれません。

@ethertank
Copy link
Contributor Author

ローカルで確認する限り、かかる変更による問題は発生していないように思います。

おそらく誤検知ではなく、実際に Shift-JIS になっています。
差分が確認しにくいですが、このリクエストを通していただければ混在の問題の残りは解決すると思います。
もしよろしければこのまま通していただく思います。

それでは問題があるという事でしたら、先んじて残りのものの変換をお願いします。

@tetsuharuohzeki
Copy link

おそらく誤検知ではなく、実際に Shift-JIS になっています

shift-jisもutf-8も、英数字部分はascii形式の文字コードの集合です。プラットフォーム・エディタによってデフォルトの文字コードが違う為、shift-jisとして解釈されているのだと思います。

pull requestですが、conflictが発生しているのでこのままではマージが出来ません。rebaseまたはマージによる競合の解決をお願いします。

@tetsuharuohzeki
Copy link

確認おくれてすみません。
うーん、、、まだ自動マージが可能な状態ではないみたいです……

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants