Closed
Bug 225749
Opened 21 years ago
Closed 19 years ago
Clean-up in spellchecker code
Categories
(Core :: Spelling checker, defect)
Core
Spelling checker
Tracking
()
RESOLVED
FIXED
People
(Reporter: mvl, Assigned: mvl)
Details
Attachments
(2 files, 1 obsolete file)
17.59 KB,
patch
|
dwitte
:
review+
alecf
:
superreview+
|
Details | Diff | Splinter Review |
9.98 KB,
patch
|
timeless
:
review+
alecf
:
superreview+
|
Details | Diff | Splinter Review |
I came across a lot of code in spellchecker that doesn't follow mozilla coding guidelines, or is just 'not nice' in general. I'm planning to put some patches in this bug to clean up random places. This is really just clean up, no change of functionality or anything.
Assignee | ||
Comment 1•21 years ago
|
||
nsIEditorSpellcheck uses const &nsString, which makes it hard to pass a substring for example.
Assignee | ||
Updated•21 years ago
|
Attachment #135508 -
Flags: superreview?(kinmoz)
Attachment #135508 -
Flags: review?(dwitte)
Assignee | ||
Comment 2•21 years ago
|
||
No need for code duplication :)
Assignee | ||
Comment 3•21 years ago
|
||
Comment on attachment 135663 [details] [diff] [review] make reading files use nsILineInputStream Come to think of it, this is wrong. the file we are reading isn't utf8 or utf16, but something else, like ISO-8859-1 And it should be stored in memory in the same encoding (not ascii as i do here)
Attachment #135663 -
Attachment is obsolete: true
Comment 4•21 years ago
|
||
Comment on attachment 135508 [details] [diff] [review] make nsIEditorSpellcheck use nsAString (Checked in) >Index: editor/composer/src/nsEditorSpellCheck.cpp >=================================================================== > nsEditorSpellCheck::CheckCurrentWord(const PRUnichar *aSuggestedWord, >+ PRBool *aIsMisspelled) > { > nsAutoString suggestedWord(aSuggestedWord); > DeleteSuggestedWordList(); >+ return mSpellChecker->CheckWord(nsDependentString(aSuggestedWord), >+ aIsMisspelled, &mSuggestedWordList); remove the |nsAutoString suggestedWord|. > nsEditorSpellCheck::ReplaceWord(const PRUnichar *aMisspelledWord, >+ const PRUnichar *aReplaceWord, >+ PRBool allOccurrences) > { >- nsAutoString misspelledWord(aMisspelledWord); > nsAutoString replaceWord(aReplaceWord); >+ return mSpellChecker->Replace(nsDependentString(aMisspelledWord), >+ nsDependentString(aReplaceWord), allOccurrences); here too >Index: extensions/spellcheck/src/mozSpellChecker.cpp >=================================================================== > NS_IMETHODIMP >+mozSpellChecker::CheckWord(const nsAString &aWord, PRBool *aIsMisspelled, nsStringArray *aSuggestions) > { > PRUnichar **words; > nsAutoString temp; > >+ mSpellCheckingEngine->Suggest(PromiseFlatString(aWord).get(), &words, &count); > for(i=0;i<count;i++){ > temp.Assign(words[i]); > aSuggestions->AppendString(temp); remove the |nsAutoString temp|, and just do this: aSuggestions->AppendString(nsDependentString(words[i])); > NS_IMETHODIMP >+mozSpellChecker::GetCurrentDictionary(nsAString &aDictionary) > { > nsXPIDLString dictname; > nsresult res; > res=mSpellCheckingEngine->GetDictionary(getter_Copies(dictname)); > if(NS_SUCCEEDED(res)) >+ aDictionary = dictname; remove the |if(NS_SUCCEEDED(res))|, it's unnecessary. (we should probably propagate the failure code too, but *shrug*) >Index: extensions/spellcheck/src/mozSpellChecker.h >=================================================================== sigh, when are we going to have an idl instead of redeclaring all this crap? ;) r=dwitte with nits fixed.
Attachment #135508 -
Flags: review?(dwitte) → review+
Assignee | ||
Updated•21 years ago
|
Attachment #135508 -
Flags: superreview?(kinmoz) → superreview?(alecf)
Assignee | ||
Comment 5•21 years ago
|
||
Comment on attachment 135508 [details] [diff] [review] make nsIEditorSpellcheck use nsAString (Checked in) dwitte: thanks for the review, i will fix those nits. alecf, if you sr, consider those nits fixed :)
Comment 6•21 years ago
|
||
Comment on attachment 135508 [details] [diff] [review] make nsIEditorSpellcheck use nsAString (Checked in) don't forget to rev the IID for nsISpellChecker and any other interfaces you change! sr=alecf as long as you rev the IID.
Attachment #135508 -
Flags: superreview?(alecf) → superreview+
Assignee | ||
Comment 7•21 years ago
|
||
This patch fixes the comments in the .idl files to use javadoc style, and the someMethod style for methods (instead of SomeMethod). I will rev the IID's when checking in, but not now, as I am planning to change the interfaces some more. (does the capitalization change require a IID change anyway?)
Assignee | ||
Updated•21 years ago
|
Attachment #136114 -
Flags: superreview?(alecf)
Attachment #136114 -
Flags: review?(timeless)
Comment 8•21 years ago
|
||
Comment on attachment 136114 [details] [diff] [review] fix comments in idl files (Checked in) sr=alecf assuming we've made sure there's no JS that needs to be updated! sr=alecf
Attachment #136114 -
Flags: superreview?(alecf) → superreview+
Assignee | ||
Updated•21 years ago
|
Attachment #135508 -
Attachment description: make nsIEditorSpellcheck use nsAString → make nsIEditorSpellcheck use nsAString (Checked in)
Comment 9•21 years ago
|
||
This might have caused bug 229215.
Attachment #136114 -
Flags: review?(timeless) → review+
Assignee | ||
Updated•21 years ago
|
Attachment #136114 -
Attachment description: fix comments in idl files → fix comments in idl files (Checked in)
Component: Editor: Core → Spelling checker
QA Contact: bugzilla → core.spelling-checker
Assignee | ||
Comment 10•19 years ago
|
||
I'm not planning any other cleanups
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•