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

bugfix Emacs TAGS format #43

Merged
merged 2 commits into from
Sep 28, 2018
Merged

bugfix Emacs TAGS format #43

merged 2 commits into from
Sep 28, 2018

Conversation

fommil
Copy link
Contributor

@fommil fommil commented Sep 27, 2018

fixes #42

This more correctly matches the output that I see from something like exuberant-ctags, i.e. https://stackoverflow.com/questions/1990579

, posLine = aiLine
, posOffset = Offset aiAbsPos
, posPrefix = aiPrefix
, posText = aiPrefix <> (fromString $ show $ unLine aiLine)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just a placeholder...

Copy link
Contributor Author

@fommil fommil Sep 27, 2018

Choose a reason for hiding this comment

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

we have the aiInput, so it should be a case of taking that position to the next newline.

@fommil fommil changed the title [WIP] bugfix for Emacs TAGS format bugfix Emacs TAGS format Sep 27, 2018
@fommil
Copy link
Contributor Author

fommil commented Sep 27, 2018

It is late, I will fix the tests in the morning... but I have confirmed that this fixes the problem 😄

I suspect there is a similar bug in hasktags as I seen the same behaviour there also.

increaseLine :: Line -> Line
increaseLine (Line n) = Line $! n + 1

data SrcPos = SrcPos {
posFile :: !FilePath
, posLine :: {-# UNPACK #-} !Line
, posOffset :: {-# UNPACK #-} !Offset
-- | No need to keep prefix strict since most of the prefixes will not be
-- used.
, posPrefix :: Text
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this field of the position is not used anywhere, I think it can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all these fields are now used.

, posLine = aiLine
, posOffset = Offset aiAbsPos
, posPrefix = aiPrefix
, posText = aiPrefix <> (Text.takeWhile (/= '\n') aiInput)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be nice to not populate this field if aiTrackPrefixes is False. I.e. when we're generating tags for vim - there's no reason to keep lots of lines around which will not be consumed in the end.

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's a lazy field in the data type, so we get that anyway for free, or do I misunderstand ?

@fommil
Copy link
Contributor Author

fommil commented Sep 28, 2018

@sergv @elaforge I have updated the PR so the tests now pass.

I can also confirm that exuberant-ctags generates this sort of thing

data RealWorld a where�RealWorld103,2929

the current release of fast-tags generates

data RealWorld�RealWorld102,103

And this patch produces identical output to exuberant-ctags

@elaforge
Copy link
Owner

Looks good to me, merging.

Thanks for giving emacs some attention, it needed it!

@elaforge elaforge merged commit 8a0e334 into elaforge:master Sep 28, 2018
@elaforge
Copy link
Owner

BTW I'll do a cabal release soon unless you have some other change in mind.

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.

incorrect emacs tag format
3 participants