Skip to content

Clarify string coercion errors #958

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 18, 2021
Merged

Clarify string coercion errors #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90ee508
Clarify string coercion errors
layus Jun 17, 2021
4a10f09
Introduce a stringParts helper
layus Jun 17, 2021
46d6d61
Merge branch 'master' into string
Anton-Latukha Jul 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Nix/Convert.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ instance ( Convertible e t f m
(M.lookup "outPath" s)
_ -> stub

fromValue = fromMayToValue $ TString mempty
fromValue = fromMayToValue $ TString HasContext
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This deserves a proper discussion. It seems that in some places the code was relying on ToString instance to generate strings with no context.

This changes the error message to reflect that strings with context are accepted.

Now, if the code was really relying on that assumption, it is still broken.

Copy link
Collaborator

@Anton-Latukha Anton-Latukha Jul 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

TString? Or string class hub ToString from relude. In the second case - so far I just worked on strings that already were spawned in a regular way.

If to preserve context, or not, - I have no idea.
If the context is beneficial - of course, it should be preserved.

If there are really literal creations of the strings and they better to have context - then lets do it.

Personally, I am bothered & occupied with that we have String, strings of text of different data types, Nix String as a type of expression, Expression, StringContext, NixString. It seems for me that some of those need to be renamed/abstracted better.

===

I'd asked maybe a straight literal stream of consciousness of what you were doing, checking, understood, and found out, and what is your decision on it.

===

Lets preserve context where it seems to be in place, it would be a bit more memory footprint, but we so far not made solid space profiling. We always can drop the context.


instance Convertible e t f m
=> FromValue ByteString m (NValue' t f m (NValue t f m)) where
Expand Down
7 changes: 1 addition & 6 deletions src/Nix/Eval.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,14 +467,9 @@ assembleString
. (MonadEval v m, FromValue NixString m v)
=> NString (m v)
-> m (Maybe NixString)
assembleString =
fromParts .
\case
Indented _ parts -> parts
DoubleQuoted parts -> parts
assembleString = fromParts . stringParts
where
fromParts xs = (mconcat <$>) . sequence <$> traverse go xs

go =
runAntiquoted
"\n"
Expand Down
4 changes: 4 additions & 0 deletions src/Nix/Expr/Types.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,10 @@ paramName :: Params r -> Maybe VarName
paramName (Param n ) = pure n
paramName (ParamSet _ _ n) = n

stringParts :: NString r -> [Antiquoted Text r]
stringParts (DoubleQuoted parts) = parts
stringParts (Indented _ parts) = parts

stripPositionInfo :: NExpr -> NExpr
stripPositionInfo = transport phi
where
Expand Down
4 changes: 2 additions & 2 deletions src/Nix/Value.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,8 +753,8 @@ describeValue =
TFloat -> "a float"
TBool -> "a boolean"
TNull -> "a null"
TString NoContext -> "a string"
TString HasContext -> "a string with context"
TString NoContext -> "a string with no context"
TString HasContext -> "a string"
TList -> "a list"
TSet -> "an attr set"
TClosure -> "a function"
Expand Down