Skip to content

Conversation

czipperz
Copy link
Contributor

No description provided.

(member char '(nil ?\t ?\n ?\ )))
(defun haskell-ds-whitespace-p ()
"Test if PT is at a whitespace character."
(equal (string-to-syntax " ") (syntax-after (point))))
Copy link
Contributor

Choose a reason for hiding this comment

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

You had nil here before to handle eobp. Is this needed or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It needs to have eolp or'd to it.

;; is cursor at begging, inside, or end of comment
(let ((fontfaces (get-text-property (or pt
(point)) 'face)))
(let ((fontfaces (get-text-property (point) 'face)))
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't add comment on a line that follows, so I'm putting it here: comments in haskell have structure and we use more faces than just font-lock-comment-face: haskell-pragma-face, haskell-liquid-haskell-annotation-face, haskell-literate-comment-face. Sometimes we also add (:weight bold) and (:slant italic).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The weight and slant already work. I can add these other faces to the member test if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, they have to be there. Otherwise declscan will not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the help.

(setq r nil))
(forward-char))
r))
"Test if all characters from `point' to `end-of-line' pass `haskell-ds-comment-p'."
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is horrible. The way to find end of current comment is to use (parse-partial-sexp (point) (point-max) nil nil (syntax-ppss) 'syntax-table). Examples are in haskell-font-lock.el.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only usage of this function is to go back to the end of a definition. That requires going through all lines that are composed only of white space and comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Use (parse-partial-sexp (point) (point-max) nil nil (syntax-ppss) 'syntax-table) for this purpose.

@gracjan
Copy link
Contributor

gracjan commented Nov 15, 2016

Do you want me to merge as is and fix the functionality later? Or fix the functionality in place while we are at it?

@czipperz
Copy link
Contributor Author

czipperz commented Nov 15, 2016

I'll fix it tonight.

It was never used with the optional custom point, so just removed it.
There was also a bug where the custom point wasn't even being used, that
this fixed.
Copy link
Contributor

@gracjan gracjan left a comment

Choose a reason for hiding this comment

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

comment

@gracjan
Copy link
Contributor

gracjan commented Nov 16, 2016

This new github review workflow is confusing. Anyway, since we are chanigng decl-scan we should make it better in those two places I mentioned.

@geraldus
Copy link
Contributor

This new github review workflow is confusing

The most confusing for me is Comment option of review. I supposed that it should simply add comment to review, but instead it submits review as simple comments :D

@czipperz
Copy link
Contributor Author

I'm going to close this pull, fix it up and reopen it. Thanks for the suggestions @gracjan

@czipperz czipperz closed this Nov 16, 2016
@gracjan
Copy link
Contributor

gracjan commented Nov 17, 2016

You can reopen and force-push new set of commits. Github is pretty good at sorting out what has happened.

@czipperz
Copy link
Contributor Author

czipperz commented Nov 17, 2016

That's the plan.

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 this pull request may close these issues.

3 participants